Skip to content

Storage: GET {base}/_local/file/:key is built in three places but mounted nowhere #3641

Description

@os-zhuang

Found while ledgering the autonomously-mounted storage surface for #3636 (route audit tranche 3).

Three call sites build a _local/file/:key URL and hand it out as a real download link:

SiteWhat it does
packages/services/service-storage/src/local-storage-adapter.ts:270getPresignedUpload() returns it as the descriptor's downloadUrl
packages/services/service-storage/src/storage-routes.ts:530GET /files/:fileId/url falls back to it when the adapter has neither getPresignedDownload nor getSignedUrl
packages/services/service-storage/src/storage-routes.ts:570GET /files/:fileId (the 302) — same fallback

No registrar mounts that path.registerStorageRoutes mounts _local/raw/:token (PUT + GET) and nothing else under _local; a repo-wide search for _local/file returns only the three builders above. So whenever one of these values is followed, it 404s.

This is the same shape as the bugs tranche 1 and 2 closed (analytics.explain calling /explain, analytics.meta calling /meta/:cube#3584): a URL that exists in code, looks right, and is served by nothing.

Why it hasn't bitten yet

Both live adapters dodge it. LocalStorageAdapter and the S3 adapter each implement getPresignedDownloadandgetSignedUrl, so the storage-routes.ts fallback branch is unreachable for them, and nothing in-tree reads PresignedUploadDescriptor.downloadUrlPOST /upload/presigned builds its own downloadUrl: {base}/files/{fileId}/url rather than passing the adapter's through. It is latent, not live: a third-party adapter implementing only upload/download gets dead links out of both download endpoints.

Decision needed

Either mount it, or delete it — the ledger should not have to carry a route that half-exists:

  1. Delete (probably right). Point the storage-routes.ts fallback at the same {base}/files/:fileId redirect the rest of the system uses as its stable URL, and drop the unread downloadUrl from getPresignedUpload's descriptor. Nothing in-tree loses a capability.
  2. Mount it. Add GET {base}/_local/file/:key serving bytes by key. Note this would be an unsigned, key-addressed read — the reason the _local/raw/:token pair is HMAC-token-gated in the first place — so it would need the same authorizeFileRead treatment the /files/* endpoints get, and probably shouldn't exist in that form at all.

Recommendation: option 1.

Tracked as a follow-up rather than folded into #3636, which is scoped to giving mounted routes reviewed dispositions. The tranche-3 ledger notes the absence explicitly (storage-route-ledger.ts header) so the guard doesn't read as "this URL was audited and approved."

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions