feat(storage): add provider-neutral object storage package (#106)

[kauandotnet]

Pull Request

Summary

Adds the preview @concepta/rockets-storage package: a provider-neutral object
storage contract, named NestJS stores, structured signed transfers, fail-closed
capability handling, hardened provider adapters, and reusable testing support.

Closes #106.

Related to #86 and #94, but does not close either. This PR supplies the storage
runtime that application services and operation handlers can inject; it does
not add inbound multipart routes or bind PR #94's
FILE_STORAGE_SERVICE_TOKEN.

Changes

• Added a framework-neutral StorageDriver and StorageClient with streaming
  reads/writes, explicitly bounded convenience reads, metadata, range reads,
  list/search, copy/move, bulk operations, resumable uploads, conditional
  operations, and structured signed transfers.
• Added stable storage error codes, strict opaque ETag validation, cancellation,
  timeouts, retries, plugins, and exact provider capabilities. Unsupported
  guarantees fail closed rather than being approximated with racy operations.
• Added StorageModule, StorageService, @InjectStorage(), default and named
  stores, forRoot() / forRootAsync(), and feature-scoped sync/async
  registration. Async stores support factory, class, and existing providers.
• Added cross-store transfer and sync workflows with dry-run, prune, and
  destination-prefix safeguards.
• Added explicit Files SDK, filesystem, runtime-provider, S3/S3-compatible, and
  testing entry points. Native provider SDKs remain optional and do not leak
  through the package root or rockets-core.
• Hardened filesystem and S3-backed behavior, including AWS and Cloudflare R2
  profiles and read-only defaults for unverified custom S3 endpoints.
• Added an in-memory driver, reusable provider conformance contract, an
  always-on filesystem conformance suite, gated live AWS/R2/custom suites, and
  a real NestJS HTTP end-to-end test for named-store injection.
• Integrated the package with workspace builds, alpha releases, public API
  reports, documentation, release-readiness CI, and clean packed-consumer
  verification.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update
• Refactoring
• Dependency update

Verification

• yarn build
• yarn api:report:update - 17 entry points and 7 API report tests
• yarn typecheck:spec
• yarn test - 119 files and 1,219 tests passed
• yarn test:e2e - 54 files; 450 tests passed and 3 live cloud
  conformance suites skipped
• yarn lint:all
• Node 20.19 storage matrix - 177 unit tests and 28 end-to-end tests
  passed; 3 live cloud suites skipped
• yarn release:packages - 7 stable assertions, 95 public artifact
  targets, all workspaces dry-packed, and all 7 public packages verified in
  clean CommonJS, ESM, TypeScript, NestJS, legacy-resolution, and
  peer-minimal consumers
• git diff --check

Live cloud conformance was not run. It is opt-in, requires disposable provider
fixtures, and remains skipped in the normal test matrix. The full filesystem
provider contract runs on every end-to-end test execution.

Scope and Compatibility

• Supported runtimes are Node.js >=20.19.0 <21 or >=22.12.0. This exact
  range is required for synchronous require() support across the ESM
  dependency graph; Node 21 and Node 22.0-22.11 are intentionally excluded.
• The root, /core, and /files-sdk entry points support legacy TypeScript
  Node10 resolution. Provider and testing subpaths require node16, nodenext,
  or bundler resolution because upstream provider packages expose modern
  export maps only.
• The /core entry point has no NestJS runtime or type dependency.
• Object keys, metadata persistence, tenancy, authorization, retention,
  scanning, transformation, and product encryption policy remain application
  concerns.
• Provider-level resumable uploads do not add HTTP multipart/form-data
  parsing, and byte-range support does not add a storage HTTP gateway.

Checklist

• My code follows the existing patterns in the codebase
• I have updated relevant documentation
• I have added tests for new functionality

[tnramalho]

Review

I ran the full gate locally on this branch: build, test (177), test:e2e
(28 passed + 3 skipped), api:report:check-built, lint:all, and
typecheck:spec — all green. The PR's verification claims hold up.

A lot here is genuinely good, and worth saying first: the capability gating is
fail-closed for real; the filesystem driver is properly hardened (symlink
checked per segment, nlink, reserved suffixes, 0o700); the presigned upload
binds content-length-range + eq $Content-Type and uses
signableHeaders: ['content-type'] on the PUT, so content type is part of the
signature rather than a decorative header; provider errors are sanitized so
bucket, key, request id, and credentials do not cross the boundary; and
verify-packed-consumer covering all 7 entry points across CJS/ESM/Node10 is
above the bar for this repo.

The items below are what I think should land before merge/publish.

---

Publish blockers (npm)

1. signDownload ignores the expiry the application asked for

The S3 adapter declares, itself, that it cannot expire a URL when
publicBaseUrl is set:

// s3/index.ts:908
signedDownloadPolicy: Object.freeze({
  expiresIn: constructionMetadata !== undefined && !publicBaseUrlConfigured,
})

But nothing reads that capability. StorageClient.signDownload
(storage.client.ts:718) and FilesSdkStorageDriver.signDownload
(files-sdk.driver.ts:1885) delegate straight through. The capability is
published in capabilities (files-sdk.driver.ts:1457) and never consulted —
grep confirms there is no enforcement point.

Effect: signDownload(key, { expiresIn: 300 }) returns, with no error, a
permanent public URL to a private object.

signUpload does exactly the right thing in the equivalent situation — it
throws NOT_SUPPORTED when the profile cannot enforce
contentType/sizeRange (s3/index.ts:823-850) — and the README promises that
behavior generally ("A requested guarantee that the selected provider cannot
enforce fails with NOT_SUPPORTED before a URL is minted", README:265). The
download side just needs to mirror it.

Suggestion: if expiresIn was requested and
capabilities.signedDownloadPolicy?.expiresIn !== true, throw NOT_SUPPORTED.
Plus an e2e proving it: with publicBaseUrl configured,
signDownload(key, { expiresIn }) rejects.

2. Signed downloads have no validation, no default, and no ceiling

signUpload validates through assertSignedUploadOptions
(storage.client.ts:101). signDownload only calls assertKey — there is no
assertSignedDownloadOptions. So a negative, zero, or 10**9 expiresIn goes
straight to the SDK (SigV4 fails above 604800), and with no expiresIn the
expiry is the Files SDK default, which this package neither documents nor
verifies.

3. "verified" here means "went through the factory", not "tested against the provider"

defineS3ProviderProfile (provider-profile.ts:50-135) only checks that the
name is a string and the booleans are booleans.
assertVerifiedS3ProviderProfile only checks the WeakSet marker. The three
suites that would perform actual verification are describe.skip with a
placeholder (__e2e__/provider-conformance.e2e-spec.ts:244) — those are the "3
skipped" — and the PR itself states live conformance was never run.

Meanwhile the README (~line 295) says "conditional behavior is unlocked only by
a verified provider profile", and AWS_S3_PROVIDER_PROFILE is applied by
inference with no opt-in (s3/index.ts:770-775) — nobody chooses to trust it,
the default trusts it.

What makes this bite is that these claims unlock exactly the operations whose
failure mode is not an error but a silently lost guarantee:
conditionalCopyDestination.atomicWithSource: true and
conditionalMultipartCompletion (s3/index.ts:88-102). An application calls
promote() believing it has compare-and-swap and can get an overwrite.

Cheap fix: switch the vocabulary to declared, and mark in the README that
AWS_S3_PROVIDER_PROFILE / CLOUDFLARE_R2_PROVIDER_PROFILE are declared and
not executed against the provider — or run the AWS suite once against a
disposable bucket and cite the result, which settles it for good.

---

Merge blockers (cheap, same PR)

4. StorageError reaches the application without store, operation, or key

This is not cosmetic — it is the intended design broken. sanitizedStorageError
(files-sdk.driver.ts:683) drops those fields on purpose, expecting
StorageClient.#execute to put them back. But #execute
(storage.client.ts:856) passes {key, operation, store} to
normalizeStorageError, which does if (isStorageError(error)) return error;
(storage.error.ts:116) and discards all of it.

client = new StorageClient('media', createMemoryStorageDriver())
await client.head('missing.txt')
-> code: NOT_FOUND | store: undefined | operation: undefined | key: undefined

It is also inconsistent: errors thrown by the client itself (NOT_SUPPORTED)
do carry store, driver errors do not. In a multi-store app you cannot tell
which store failed, and the sanitized message is generic. No test covers these
fields.

Fix: fill in the missing fields in normalizeStorageError instead of returning
early, without overwriting what the driver already set.

5. sync() re-uploads everything when the two stores use different drivers

sync() requires different stores (assertDifferentStores), but the default is
compare: 'etag'. Each driver generates ETags with its own algorithm (fs uses
sha256, memory uses another, S3 uses md5) and the package treats ETags as opaque
tokens by design. For any heterogeneous pair the comparison can never match.

Probe against dist (memory → fs, identical content, repeated runs):

run1: {"uploaded":["a.txt"],"skipped":[]}
run2: {"uploaded":["a.txt"],"skipped":[]}   <- should have skipped
run3 (compare:'size'): {"uploaded":[],"skipped":["a.txt"]}
mem etag: 6d61366e
fs  etag: 2aae6c35c94fcfb4

The tests miss it because every sync case uses two memory drivers
(storage.service.spec.ts:61+) — same algorithm on both sides.

Suggestion: default to 'size', or fail when driverName differs and compare
was not set explicitly. Plus a heterogeneous sync test.

---

Decisions I think need an explicit sign-off

6. Zero consumers, and 204 exports frozen

grep -rn "rockets-storage" packages examples outside the package itself
returns nothing. No example app, no integration with core/server/server-auth,
and the PR states it does not close #86 nor bind #94's
FILE_STORAGE_SERVICE_TOKEN — and #94 is still open/WIP.

That is 7 new entry points and 204 exported names (the monorepo goes from 10 to
17 entry points), frozen in public-api-reports.json and marked major in
.yarn/versions/alpha.yml, so it publishes on the next alpha. Pre-1.0, with no
consumer proving the design.

Suggestion: one real use under examples/ before freezing the surface — even
just an avatar operationResource.

7. files-sdk is a hard runtime dependency, and it leaks into the "neutral" core

files-sdk@2.2.3 (exact pin, unscoped, single maintainer, ~3.5 months old) is a
dependencies entry, not an optional peer. It brings commander@^15,
aws4fetch, picomatch, and safe-regex2 as hard deps into every consumer's
tree, and ships a CLI bin. (@modelcontextprotocol/sdk is optional, so that
one stays out.) It is a "unified storage + AI agent tooling + CLI" package, with
45+ optional peers ranging from React/Vue/Svelte/Next to @openai/agents and
@anthropic-ai/claude-agent-sdk — not a lean storage core.

And /core, presented as the neutral contract, imports it at runtime:
storage-upload-control.ts:1 does import { UploadControl } from 'files-sdk',
re-exported by core/index.ts. validResumableSession()
(storage-upload-control.ts:58-122) is a switch over 17 concrete provider
slugs with default: return false — a new provider in files-sdk means resume
tokens are rejected until someone edits the "neutral" layer. And
StorageProviderConfig is a direct alias of LoadFilesOptions, so a type change
upstream is a breaking change here.

I am not asking to swap the engine — the exact pin is coherent with that switch.
But it is worth stating in the README that /core is neutral with respect to
NestJS and native SDKs, not with respect to files-sdk.

8. The only ESM package in a CJS monorepo

"type": "module" against commonjs in the other six, with require pointing
at the same ESM file and engines >=20.19.0 <21 || >=22.12.0. The interop is
actually proven (verify-cjs.cjs require()s all 7 subpaths from the packed
tarball on Node 20.19.0 in CI), so this is not broken — it is a choice. I just
think "adopting storage raises the app's Node floor" deserves a conscious
decision rather than riding along inside a 23k-line PR.

---

Minor (follow-up)

• rangeHeader does not validate (s3/index.ts:411): start: -5 becomes
  bytes=-5-, which in HTTP is a suffix range ("last 5 bytes") — inverted
  semantics, silently. It is only safe today because StorageClient validates
  first, but createS3StorageDriver is public surface and can be used directly.
• publicBaseUrlConfigured is derived from the config object
  (provider/index.ts:132-143), not from the resolved adapter. If the Files SDK
  loader accepts a public base URL via env var, the flag stays false while the
  adapter serves public URLs — the capability would lie in the dangerous
  direction. Worth confirming with listStorageProviderEnvVars('s3').
• limit on transfer/sync (storage.types.ts:425,460) is a page size
  forwarded to listAll, not a cap on objects, and has no jsdoc. On an
  operation with prune, the name misleads.
• The README does not document transfer()/sync()/prune — just a
  one-line mention (README:230). Those are 216 of StorageService's 367 lines,
  and prune deletes objects.
• Double branding in s3/provider-profile.ts: the symbol written via
  Object.defineProperty (line 216) is never read — only the WeakSet (223) is
  checked. The symbol earns its keep as a nominal type marker; the runtime write
  is dead code (AGENTS.md rule 10). The WeakSet also locks identity per realm,
  which is the opposite philosophy to isStorageError, which duck-types
  cross-realm on purpose.
• storage.module.ts:173: definition.useFactory as unknown as (...) with
  no comment (AGENTS.md rule 9). In the useClass/useExisting branches the
  factory parameter falls back to implicit any.
• internal/settle-many.ts: resolveConcurrency only runs on the
  non-stopOnError path, so { concurrency: -1, stopOnError: true } passes
  unvalidated. And if (item === undefined) continue drops the item silently —
  it lands in neither results nor errors.
• The Nest module e2e is a single happy path (79 lines). There is no e2e for
  forRootAsync, multi-store, StorageService.use(), or shutdown/close().
• Merge order: #104: one schema engine (upstream alpha.9, zod everywhere) #105 also rewrites api/public-api-reports.json and
  api/public-api-policy.md. Whichever lands second should regenerate rather
  than hand-resolve the JSON conflict.

[tnramalho]

Review follow-up — the four blockers are fixed

Pushed in 0cb5bef. Items 1, 2, 4 and 5 from the review are addressed; item 8
(merge order) resolved itself. Items 3, 6 and 7 are unchanged and still need a
call — see the bottom.

1 + 2. signDownload ignored the requested expiry, and validated nothing

StorageClient.signDownload now reads the capability the S3 adapter was
already publishing. Requesting expiresIn on a store that does not advertise
signedDownloadPolicy.expiresIn: true throws NOT_SUPPORTED instead of
minting a URL that ignores it — which closes the reported case: an S3 store
with publicBaseUrl reports expiresIn: false and was returning a permanent
public URL for a private object.

This is broader than the publicBaseUrl case, and worth being explicit
about. Only the S3 adapter declares signedDownloadPolicy. GCS, Azure, R2
and other S3-compatible endpoints reached through the runtime provider entry
point, the filesystem driver, and any third-party driver leave it undeclared,
so they now reject expiresIn too. signDownload without expiresIn is
unaffected everywhere.

I went strict rather than "throw only when a policy says false" because
upstream says the permissive reading is not available. From
files-sdk's own SignedUrlCapability.supported doc:

whether such a URL honors expiresIn exactly is a separate, per-provider
detail — some providers pin the lifetime server-side and ignore the request

There is no truthful automatic signal, so fail-closed is the only honest
default. The cost is that signed-download expiry is an S3-only feature until
each adapter declares the guarantee — the same provider-profile mechanism
already used for signed uploads. Worth its own issue.

Validation: expiresIn must be a positive safe integer, and must not exceed
the lower of two ceilings — signedDownload.maxExpiresIn, which the provider
enforces in code (Dropbox pins temporary links to 4 hours), and a new
signedDownloadPolicy.maxExpiresIn, which is the documented limit of the
adapter's signature format that the provider does not enforce. S3 declares
SigV4's 604800 there; files-sdk explicitly says that ceiling is "documented,
not enforced here". Both signed-URL validators now carry store, operation
and key.

Omitting expiresIn falls back to the provider default. The package still
does not set it — it now says so, in the type's jsdoc and the README, instead
of leaving it as a surprise.

One correction while doing this: I first removed StorageSignedUrlCapability.maxExpiresIn
thinking it was dead (rule 10). It is not — files-sdk populates it at
runtime. Restored; the only public-API change is the added field.

4. StorageError reached the application without store, operation, key

Fixed where you pointed: normalizeStorageError filled in the missing fields
instead of returning early. Fields the driver did set stay authoritative, the
stack is preserved, and an error that needs no context is returned by identity
— which matters for settle-many, since it calls normalizeStorageError with
no options at all.

client = new StorageClient('media', createMemoryStorageDriver())
await client.head('missing.txt')
-> code: NOT_FOUND | store: 'media' | operation: 'head' | key: 'missing.txt'

Covered by a test that is impossible to pass pre-fix.

5. sync() re-uploaded everything across different drivers

sync() now throws INVALID_ARGUMENT when the two stores' drivers differ and
compare was not set explicitly. I picked failing over defaulting to 'size':
size-only comparison silently skips same-length content changes, which trades
a loud bug for a quiet one.

The test is the memory → filesystem e2e you asked for
(src/__e2e__/cross-driver-sync.e2e-spec.ts). Its last assertion is the point:

expect(source.etag).not.toBe(destination.etag);

so it proves the premise, not just the throw.

The discriminator is driver name, which is deliberately conservative in one
direction: S3-compatible endpoints under different driver names (MinIO,
Wasabi, Spaces) do produce comparable ETags, and those callers now have to
pass compare: 'etag' explicitly. Documented in the README, along with the
converse — matching driver names are not a guarantee either, since two S3
buckets with different multipart part sizes yield different ETags for
identical content.

8. Merge order

Resolved. #105 landed first; I merged main and regenerated
api/public-api-reports.json rather than hand-resolving it. Regenerating
again after the merge produced no diff.

Docs

The README gained the signed-download rules and a real "Cross-store transfer
and sync" section — transfer(), sync(), compare, and prune, which
deletes objects and had one line of coverage.

Three claims I wrote in the first pass were wrong and are corrected: the
filesystem driver emits a truncated sha1, not sha256; Azure declares no
maxExpiresIn (only Dropbox does), so the earlier "Azure clamps to 7 days"
was invented; and transfer() does not take dryRun or destinationPrefix
— those are sync()-only.

Verification

yarn build, yarn api:report:check-built (7/7), yarn typecheck:spec,
yarn test (1,373), yarn test:e2e (508 passed + 3 live suites skipped),
yarn lint:all — all green on the merged branch.

Still open — your call, not code

• 3. "verified" still means "went through the factory". The three
  conformance suites are still describe.skip; live conformance has not been
  run. Untouched here.
• 6. Zero consumers, 204 exports, marked major. Untouched — it publishes
  on the next alpha unless something changes.
• 7. files-sdk as a hard dependency leaking into /core. Untouched.

I did not touch the minor list either, except responseContentDisposition,
which fell inside item 2. Two things worth recording from this pass:

• testing/provider-conformance.ts has no signed-download cases at all. A
  third-party adapter can declare signedDownloadPolicy.expiresIn: true and
  lie, and nothing catches it. That is the natural home for the proof.
• signUpload has no expiry-honoring capability either — the same shape as
  item 1, currently fail-open.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Es4VGiJ3j33W1cbotqVcXW