fix(service-storage): stop handing out _local/file/:key, a URL nothing mounts (#3641)

Three call sites built `${basePath}/_local/file/<key>`. No registrar has ever
mounted it, so anyone who followed one got a 404. The tranche-3 storage ledger
(#3636) recorded the URL as deliberately ABSENT and filed this; now nothing
builds it either.

Each site fixed according to what it could honestly do:

  - LocalStorageAdapter.getPresignedUpload() omits `downloadUrl` (optional on
    the descriptor). It CANNOT construct the real capability URL — that is
    keyed by sys_file.id and an adapter only ever sees the storage key. Nothing
    read the field, which is how it survived: the presigned-upload route builds
    its own downloadUrl and ignores this one, and all three real readers of
    `desc.downloadUrl` take it from getPresignedDownload, whose URL IS mounted.

  - GET /files/:fileId/url and GET /files/:fileId answer 501 NOT_IMPLEMENTED
    when the adapter has neither getPresignedDownload nor getSignedUrl, instead
    of returning/redirecting to the unmounted URL. The caller learns the
    adapter is the limitation rather than chasing a broken link.

Behaviour change is confined to adapters implementing neither capability;
LocalStorageAdapter and the S3 adapter both implement getPresignedDownload, so
no shipped path changes. A 200/302 pointing at a 404 becomes a 501 that says why.

Two conformance cases added, mutation-checked: restoring either dead URL fails
them.

service-storage: 198 passed (17 files), up from 196.

Note on typecheck: this package has no `typecheck` script and CI's TypeScript
gate runs `--filter @objectstack/spec` only. A bare `tsc --noEmit` here reports
39 pre-existing errors, 7 of them in test files this change never touches (the
repo-wide `.js`-extension style under node16 resolution). Not touched, and not
claimed as clean.

Co-Authored-By: Claude <noreply@anthropic.com>

Author: claude

.changeset/fix-unmounted-local-file-url.md

@@ -0,0 +1,35 @@
+---
+"@objectstack/service-storage": patch
+---
+
+fix(service-storage): stop handing out `_local/file/:key`, a URL nothing mounts (#3641)
+
+Three call sites built `${basePath}/_local/file/<key>`. No registrar has ever
+mounted it, so anyone who followed one got a 404. Found by the tranche-3
+storage ledger (#3636), which recorded the URL as deliberately absent and filed
+this; now nothing builds it either.
+
+Each site is fixed according to what it could honestly do:
+
+- **`LocalStorageAdapter.getPresignedUpload()`** simply omits `downloadUrl`
+  (optional on the descriptor). It cannot construct the real capability URL —
+  that is keyed by `sys_file.id`, and an adapter only ever sees the storage
+  key. Nothing read the field anyway, which is how it survived: the
+  presigned-upload route builds its own `downloadUrl`
+  (`${basePath}/files/:fileId/url`) and ignores this one, while all three real
+  readers of `desc.downloadUrl` take it from `getPresignedDownload`, whose URL
+  *is* mounted (`_local/raw/<token>`).
+
+- **`GET /files/:fileId/url` and `GET /files/:fileId`** answer **501
+  `NOT_IMPLEMENTED`** when the adapter has neither `getPresignedDownload` nor
+  `getSignedUrl`, instead of returning (or redirecting to) the unmounted URL.
+  The caller now learns the adapter is the limitation rather than chasing a
+  broken link.
+
+Behaviour change is confined to adapters implementing neither capability —
+`LocalStorageAdapter` and the S3 adapter both implement `getPresignedDownload`,
+so no shipped path changes. A 200/302 pointing at a 404 becomes a 501 that says
+why.
+
+Two conformance cases added for the new branches, and mutation-checked:
+restoring either dead URL fails them.

packages/services/service-storage/src/error-envelope.conformance.test.ts

@@ -222,6 +222,32 @@ describe('storage error envelope (#3675)', () => {
         return drive(routes, 'PUT', `${BASE}/_local/raw/:token`, { params: { token: 'x' } });
       },
     },
+    {
+      // #3641: these two used to answer 200/302 with
+      // `${basePath}/_local/file/<key>` — a URL no registrar mounts — so a
+      // caller followed a link straight into a 404. An adapter that can
+      // produce no download URL now says so.
+      name: 'download URL from an adapter with neither presigned nor signed URLs',
+      status: 501,
+      code: 'NOT_IMPLEMENTED',
+      run: async () => {
+        const store = new StorageMetadataStore(null);
+        await committedAttachment(store, 'nocap', { scope: 'user', key: 'user/nocap.bin' });
+        const routes = mount({} as any, store);
+        return drive(routes, 'GET', `${BASE}/files/:fileId/url`, { params: { fileId: 'nocap' } });
+      },
+    },
+    {
+      name: 'the redirect twin of the same adapter limitation',
+      status: 501,
+      code: 'NOT_IMPLEMENTED',
+      run: async () => {
+        const store = new StorageMetadataStore(null);
+        await committedAttachment(store, 'nocap2', { scope: 'user', key: 'user/nocap2.bin' });
+        const routes = mount({} as any, store);
+        return drive(routes, 'GET', `${BASE}/files/:fileId`, { params: { fileId: 'nocap2' } });
+      },
+    },
     {
       name: 'raw download with a forged token signature',
       status: 403,

packages/services/service-storage/src/local-storage-adapter.ts

@@ -267,7 +267,20 @@ export class LocalStorageAdapter implements IStorageService {
       method: 'PUT',
       headers: options?.contentType ? { 'content-type': options.contentType } : { 'content-type': 'application/octet-stream' },
       expiresIn,
-      downloadUrl: `${this.baseUrl}${this.basePath}/_local/file/${encodeURIComponent(key)}`,
+      // `downloadUrl` is deliberately OMITTED (it is optional on the
+      // descriptor). It used to be `${basePath}/_local/file/<key>` — a URL no
+      // registrar has ever mounted, so anyone following it got a 404 (#3641).
+      //
+      // Nothing read it, which is why it survived: the presigned-upload route
+      // builds its own `downloadUrl` (`${basePath}/files/:fileId/url`) and
+      // ignores this field, and the three real readers of `desc.downloadUrl`
+      // all take it from `getPresignedDownload`, whose URL IS mounted
+      // (`_local/raw/<token>`).
+      //
+      // Not repaired into a working URL, because this adapter cannot build
+      // one: the capability URL is keyed by `sys_file.id`, and an adapter only
+      // ever sees the storage key. Emitting nothing is honest; emitting a
+      // 404 was not.
     };
   }
 

packages/services/service-storage/src/storage-route-ledger.ts

@@ -25,8 +25,13 @@
  * SCOPE & SHAPE. Rows carry full wire paths at the DEFAULT base
  * (`/api/v1/storage`); `StorageRoutesOptions.basePath` can move the whole
  * family, and the conformance test enumerates at the same default so the two
- * stay comparable. `GET {base}/_local/file/:key` is deliberately ABSENT — three
- * call sites build that URL but no registrar has ever mounted it (#3641).
+ * stay comparable. `GET {base}/_local/file/:key` is ABSENT because nothing
+ * mounts it — and since #3641 nothing BUILDS it either. The three call sites
+ * that used to are gone: the local adapter's upload descriptor simply omits
+ * the optional `downloadUrl` (it cannot construct the real capability URL,
+ * which is keyed by `sys_file.id` rather than the storage key), and the two
+ * download routes now answer 501 when the adapter can produce no URL at all,
+ * instead of handing back one that 404s.
  *
  * This module is package-internal (not exported from the index): it is the
  * guard's data, not public API. It must stay import-free — the client-side

packages/services/service-storage/src/storage-routes.ts

@@ -543,7 +543,11 @@ export function registerStorageRoutes(
       } else if (storage.getSignedUrl) {
         url = await storage.getSignedUrl(file.key, ttl, downloadOpts);
       } else {
-        url = `${basePath}/_local/file/${encodeURIComponent(file.key)}`;
+        // See the sibling branch on `GET /files/:fileId`: no adapter capability
+        // means no download URL, and handing back an unmounted one is worse
+        // than admitting it (#3641).
+        sendError(res, 501, 'NOT_IMPLEMENTED', 'This storage adapter cannot issue download URLs');
+        return;
       }
 
       res.json({ url });
@@ -583,7 +587,13 @@ export function registerStorageRoutes(
       } else if (storage.getSignedUrl) {
         url = await storage.getSignedUrl(file.key, ttl, downloadOpts);
       } else {
-        url = `${basePath}/_local/file/${encodeURIComponent(file.key)}`;
+        // An adapter with neither `getPresignedDownload` nor `getSignedUrl`
+        // cannot produce a download URL. This used to redirect to
+        // `${basePath}/_local/file/<key>`, which no registrar mounts — a 302
+        // straight into a 404 (#3641). Say so instead: the caller learns the
+        // adapter is the limitation, rather than chasing a broken link.
+        sendError(res, 501, 'NOT_IMPLEMENTED', 'This storage adapter cannot issue download URLs');
+        return;
       }
 
       res.status(302).header('Location', url).send('');