diff --git a/.changeset/fail-closed-conditional-files-policy.md b/.changeset/fail-closed-conditional-files-policy.md new file mode 100644 index 0000000..441ddd8 --- /dev/null +++ b/.changeset/fail-closed-conditional-files-policy.md @@ -0,0 +1,8 @@ +--- +'@nestm/storage': patch +--- + +Fail conditional storage operations closed when caller-configured Files SDK +plugins, hooks, or receipts would be bypassed by native adapter extensions. +Ordinary operations continue through Files SDK while incompatible conditional +capabilities are hidden until Files SDK exposes one shared interception boundary. diff --git a/README.md b/README.md index 4fc2543..a5b855d 100644 --- a/README.md +++ b/README.md @@ -555,6 +555,41 @@ run the reusable against dedicated test credentials. Unknown endpoints are forced read-only and receive no inferred conditional capabilities. +## Files SDK responsibility boundary + +Files SDK is the upstream authority for the generic storage data plane: +provider adapters, generic CRUD, bulk and list operations, retries, transfers +and sync, its plugin pipeline, and framework-neutral gateway mechanics. +`@nestm/storage` retains the guarantees that Files SDK does not currently +provide: NestJS 12 named stores, exact native conditional/CAS capabilities, +`StorageWorkspace` permissions and limits, bounded storage errors, and +capability-scoped AI tools. + +On the alpha.8 base, ordinary `FilesSdkStorageDriver` operations already +delegate to the Files SDK pipeline. The exception is the native conditional +adapter extensions: the current Files SDK operation union does not include +them, so they cannot run through caller-configured Files plugins, hooks, or +receipts. Until Files SDK provides one interception boundary for ordinary and +conditional operations, the driver applies this interim fail-closed +compatibility rule: + +| Caller Files configuration | Ordinary operations | Conditional operations | +| ------------------------------------------------- | ------------------------------------ | -------------------------------------------------------- | +| No plugins, active hooks, or receipts | Files pipeline | Advertised when the adapter supports the exact primitive | +| One or more plugins | Files pipeline, including transforms | Hidden; direct invocation returns `NOT_SUPPORTED` | +| Any active hook | Files pipeline and hook callbacks | Hidden; direct invocation returns `NOT_SUPPORTED` | +| Receipts enabled with `true` or an options object | Files pipeline and receipts | Hidden; direct invocation returns `NOT_SUPPORTED` | + +An empty plugin list, an empty hooks object, and `receipts: false` do not trigger +the gate. NestM's internal physical-key guard does not trigger it either. When +available, direct conditional paths independently apply prefixing, the +physical-key budget, mutation read-only restrictions, default +retry/signal/timeout options, and bounded error mapping. `StoragePlugin` remains +a separate veto/observation boundary; it is not a substitute for Files body or +result transforms. This compatibility gate is intended to be removed once +native CAS can traverse the upstream operation and plugin pipeline rather than +becoming a second generic CRUD facade here. + ## Storage API `StorageClient` exposes: diff --git a/src/files-sdk/files-sdk.driver.spec.ts b/src/files-sdk/files-sdk.driver.spec.ts index 33e2e25..2802298 100644 --- a/src/files-sdk/files-sdk.driver.spec.ts +++ b/src/files-sdk/files-sdk.driver.spec.ts @@ -1,7 +1,7 @@ import { Readable } from 'node:stream'; import { inspect } from 'node:util'; -import { createStoredFile, handlers } from 'files-sdk'; +import { createStoredFile, handlers, type FilesHooks } from 'files-sdk'; import { memory } from 'files-sdk/memory'; import { @@ -658,6 +658,209 @@ describe('FilesSdkStorageDriver', () => { expect(list).not.toHaveBeenCalled(); }); + it('keeps ordinary uploads in the Files plugin pipeline and fails conditional uploads closed', async () => { + const adapter = Object.assign(memory(), { + conditionalCreate: { resultEtag: true }, + uploadConditional: vi.fn(async (key: string) => ({ + contentType: 'text/plain', + etag: 'conditional-etag', + key, + size: 10, + })), + }); + const upload = vi.spyOn(adapter, 'upload'); + const transform = vi.fn(); + const driver = createFilesSdkDriver({ + adapter, + plugins: [ + { + name: 'body-transform', + wrap: handlers({ + upload: (operation, next) => { + transform(operation.body); + return next({ ...operation, body: 'ciphertext' }); + }, + }), + }, + ], + }); + + expect(driver.capabilities.conditionalCreate).toBeUndefined(); + await expect(driver.upload('ordinary.txt', 'plaintext')).resolves.toEqual( + expect.objectContaining({ key: 'ordinary.txt' }), + ); + expect(transform).toHaveBeenCalledOnce(); + expect(transform).toHaveBeenCalledWith('plaintext'); + expect(upload.mock.calls[0]?.[1]).toBe('ciphertext'); + + await expect( + driver.uploadConditional('conditional.txt', 'plaintext', { + condition: { type: 'create' }, + }), + ).rejects.toMatchObject({ + code: StorageErrorCode.NOT_SUPPORTED, + permanent: true, + }); + expect(transform).toHaveBeenCalledOnce(); + expect(adapter.uploadConditional).not.toHaveBeenCalled(); + }); + + it.each([ + { + filesPolicy: { + plugins: [{ name: 'no-op', wrap: handlers({}) }], + }, + policyName: 'a nonempty plugin list', + }, + { + filesPolicy: { hooks: { onAction: vi.fn() } }, + policyName: 'an active action hook', + }, + { + filesPolicy: { hooks: { onError: vi.fn() } }, + policyName: 'an active error hook', + }, + { + filesPolicy: { hooks: { onRetry: vi.fn() } }, + policyName: 'an active retry hook', + }, + { + filesPolicy: { receipts: true }, + policyName: 'receipts', + }, + { + filesPolicy: { receipts: { sha256: false } }, + policyName: 'receipt options', + }, + ])( + 'hides and blocks every conditional operation with $policyName', + async ({ filesPolicy }) => { + const adapter = Object.assign(memory(), { + conditionalCopyDestination: { + atomicWithSource: true, + create: true, + replace: true, + }, + conditionalCopySource: { etag: true, version: true }, + conditionalCreate: { resultEtag: true }, + conditionalDelete: { etag: true }, + conditionalMultipartCompletion: { create: true, replace: true }, + conditionalRead: { etag: true, version: true }, + conditionalReplace: { resultEtag: true }, + deleteConditional: vi.fn(), + downloadConditional: vi.fn(), + promote: vi.fn(), + uploadConditional: vi.fn(), + }); + const driver = createFilesSdkDriver({ adapter, ...filesPolicy }); + + expect(driver.capabilities.conditionalCopyDestination).toBeUndefined(); + expect(driver.capabilities.conditionalCopySource).toBeUndefined(); + expect(driver.capabilities.conditionalCreate).toBeUndefined(); + expect(driver.capabilities.conditionalDelete).toBeUndefined(); + expect( + driver.capabilities.conditionalMultipartCompletion, + ).toBeUndefined(); + expect(driver.capabilities.conditionalRead).toBeUndefined(); + expect(driver.capabilities.conditionalReplace).toBeUndefined(); + + await expect( + driver.uploadConditional('create.txt', 'plaintext', { + condition: { type: 'create' }, + }), + ).rejects.toMatchObject({ + code: StorageErrorCode.NOT_SUPPORTED, + permanent: true, + }); + await expect( + driver.downloadConditional('read.txt', { + condition: { etag: 'current-etag' }, + }), + ).rejects.toMatchObject({ + code: StorageErrorCode.NOT_SUPPORTED, + permanent: true, + }); + await expect( + driver.deleteConditional('delete.txt', { + condition: { etag: 'current-etag' }, + }), + ).rejects.toMatchObject({ + code: StorageErrorCode.NOT_SUPPORTED, + permanent: true, + }); + await expect( + driver.promote('source.txt', 'destination.txt', { + destination: { type: 'create' }, + sourceEtag: 'source-etag', + }), + ).rejects.toMatchObject({ + code: StorageErrorCode.NOT_SUPPORTED, + permanent: true, + }); + + expect(adapter.uploadConditional).not.toHaveBeenCalled(); + expect(adapter.downloadConditional).not.toHaveBeenCalled(); + expect(adapter.deleteConditional).not.toHaveBeenCalled(); + expect(adapter.promote).not.toHaveBeenCalled(); + }, + ); + + it('keeps conditional operations compatible with explicitly inactive Files options', async () => { + const adapter = Object.assign(memory(), { + conditionalCreate: { resultEtag: true }, + uploadConditional: vi.fn(async (key: string) => ({ + contentType: 'text/plain', + etag: 'conditional-etag', + key, + size: 4, + })), + }); + const driver = createFilesSdkDriver({ + adapter, + hooks: {}, + plugins: [], + receipts: false, + }); + + expect(driver.capabilities.conditionalCreate).toEqual({ + resultEtag: true, + }); + await expect( + driver.uploadConditional('conditional.txt', 'body', { + condition: { type: 'create' }, + }), + ).resolves.toMatchObject({ key: 'conditional.txt' }); + expect(adapter.uploadConditional).toHaveBeenCalledOnce(); + }); + + it('snapshots an inactive hooks object before deciding conditional compatibility', async () => { + const adapter = Object.assign(memory(), { + conditionalCreate: { resultEtag: true }, + uploadConditional: vi.fn(async (key: string) => ({ + contentType: 'text/plain', + etag: 'conditional-etag', + key, + size: 4, + })), + }); + const hooks: FilesHooks = {}; + const driver = createFilesSdkDriver({ adapter, hooks }); + const onAction = vi.fn(); + hooks.onAction = onAction; + + await expect(driver.upload('ordinary.txt', 'body')).resolves.toMatchObject({ + key: 'ordinary.txt', + }); + await expect( + driver.uploadConditional('conditional.txt', 'body', { + condition: { type: 'create' }, + }), + ).resolves.toMatchObject({ key: 'conditional.txt' }); + + expect(onAction).not.toHaveBeenCalled(); + expect(adapter.uploadConditional).toHaveBeenCalledOnce(); + }); + it('rejects a raw undecorated s3 adapter before dispatch', () => { const adapter = s3BackedMemoryAdapter(); const upload = vi.spyOn(adapter, 'upload'); diff --git a/src/files-sdk/files-sdk.driver.ts b/src/files-sdk/files-sdk.driver.ts index 9d9db13..61a2c65 100644 --- a/src/files-sdk/files-sdk.driver.ts +++ b/src/files-sdk/files-sdk.driver.ts @@ -64,6 +64,23 @@ import { getFilesSdkUploadControl } from '../storage-upload-control.js'; export type FilesSdkDriverOptions = FilesOptions; +/** + * Interim compatibility gate until Files SDK exposes native conditional/CAS + * verbs through the same operation pipeline as its generic data plane. + */ +function hasCallerFilesOperationPolicy( + options: FilesSdkDriverOptions, +): boolean { + const hooks = options.hooks; + return ( + (options.plugins?.length ?? 0) > 0 || + typeof hooks?.onAction === 'function' || + typeof hooks?.onError === 'function' || + typeof hooks?.onRetry === 'function' || + (options.receipts !== undefined && options.receipts !== false) + ); +} + export type FilesSdkS3AdapterProvenance = 'native' | 'verified' | 'unverified'; const FILES_SDK_S3_RESERVED_EXTENSION_KEYS = [ @@ -1253,6 +1270,7 @@ export class FilesSdkStorageDriver< readonly #conditionalDelete: FilesSdkConditionalDeleteAdapter | undefined; readonly #conditionalRead: FilesSdkConditionalReadAdapter | undefined; readonly #conditionalUpload: FilesSdkConditionalUploadAdapter | undefined; + readonly #conditionalOperationsBlockedByFilesPolicy: boolean; readonly #physicalKey: FilesSdkPhysicalKeyAdapter | undefined; readonly #prefix: string; readonly #readOnly: boolean; @@ -1326,6 +1344,16 @@ export class FilesSdkStorageDriver< } const readOnly = options.readonly === true || s3Provenance?.provenance === 'unverified'; + // Evaluate caller policy before appending NestM's internal key guard below. + this.#conditionalOperationsBlockedByFilesPolicy = + hasCallerFilesOperationPolicy(options); + // Files retains the hooks object by reference. Snapshot it so an initially + // inactive object cannot be mutated after this compatibility decision and + // start observing only ordinary operations. + const hooks = + options.hooks === undefined + ? undefined + : Object.freeze({ ...options.hooks }); this.#physicalKey = physicalKeyAdapterOf(adapterForFiles); const guardedPrefix = normalizeFilesSdkPrefix(options.prefix); const plugins = [ @@ -1338,6 +1366,7 @@ export class FilesSdkStorageDriver< this.#files = new Files({ ...options, adapter: adapterForFiles, + ...(hooks !== undefined && { hooks }), plugins, readonly: readOnly, }); @@ -1375,7 +1404,8 @@ export class FilesSdkStorageDriver< rangeRead: capabilities.rangeRead, resumableUpload: !this.#readOnly && capabilities.multipart, serverSideCopy: !this.#readOnly && capabilities.serverSideCopy, - ...(this.#conditionalCopy !== undefined && + ...(!this.#conditionalOperationsBlockedByFilesPolicy && + this.#conditionalCopy !== undefined && !this.#readOnly && { ...(this.#conditionalCopy.conditionalCopySource !== undefined && { conditionalCopySource: { @@ -1389,16 +1419,19 @@ export class FilesSdkStorageDriver< }, }), }), - ...(this.#conditionalDelete !== undefined && + ...(!this.#conditionalOperationsBlockedByFilesPolicy && + this.#conditionalDelete !== undefined && !this.#readOnly && { conditionalDelete: { ...this.#conditionalDelete.conditionalDelete, }, }), - ...(this.#conditionalRead !== undefined && { - conditionalRead: { ...this.#conditionalRead.conditionalRead }, - }), - ...(this.#conditionalUpload !== undefined && + ...(!this.#conditionalOperationsBlockedByFilesPolicy && + this.#conditionalRead !== undefined && { + conditionalRead: { ...this.#conditionalRead.conditionalRead }, + }), + ...(!this.#conditionalOperationsBlockedByFilesPolicy && + this.#conditionalUpload !== undefined && !this.#readOnly && { ...(this.#conditionalUpload.conditionalCreate !== undefined && { conditionalCreate: { @@ -1463,6 +1496,9 @@ export class FilesSdkStorageDriver< invalidConditionalEtag('condition.etag', key, 'upload'), ); } + if (this.#conditionalOperationsBlockedByFilesPolicy) { + return Promise.reject(this.#conditionalFilesPolicyError(key, 'upload')); + } if (this.#readOnly) { return Promise.reject( new StorageError( @@ -1559,6 +1595,9 @@ export class FilesSdkStorageDriver< invalidConditionalEtag('condition.etag', key, 'download'), ); } + if (this.#conditionalOperationsBlockedByFilesPolicy) { + return Promise.reject(this.#conditionalFilesPolicyError(key, 'download')); + } const adapter = this.#conditionalRead; if ( adapter === undefined || @@ -1647,6 +1686,9 @@ export class FilesSdkStorageDriver< invalidConditionalEtag('condition.etag', key, 'delete'), ); } + if (this.#conditionalOperationsBlockedByFilesPolicy) { + return Promise.reject(this.#conditionalFilesPolicyError(key, 'delete')); + } if (this.#readOnly) { return Promise.reject( new StorageError( @@ -1741,6 +1783,11 @@ export class FilesSdkStorageDriver< invalidConditionalEtag('destination.etag', destinationKey, 'promote'), ); } + if (this.#conditionalOperationsBlockedByFilesPolicy) { + return Promise.reject( + this.#conditionalFilesPolicyError(sourceKey, 'promote'), + ); + } if (this.#readOnly) { return Promise.reject( new StorageError( @@ -1924,6 +1971,21 @@ export class FilesSdkStorageDriver< ); } + #conditionalFilesPolicyError( + key: string, + operation: 'delete' | 'download' | 'promote' | 'upload', + ): StorageError { + return new StorageError( + 'Conditional storage operations are unavailable while Files SDK plugins, hooks, or receipts are configured.', + { + code: StorageErrorCode.NOT_SUPPORTED, + key, + operation, + permanent: true, + }, + ); + } + #conditionalOptions< Options extends | StorageConditionalUploadOptions diff --git a/src/workspace/storage-workspace.spec.ts b/src/workspace/storage-workspace.spec.ts index 8998f04..94a7a00 100644 --- a/src/workspace/storage-workspace.spec.ts +++ b/src/workspace/storage-workspace.spec.ts @@ -8,6 +8,8 @@ import { import { tmpdir } from 'node:os'; import { join } from 'node:path'; +import { handlers } from 'files-sdk'; + import { createFsStorageDriver } from '../files-sdk/fs/index.js'; import { StorageClient } from '../storage.client.js'; import { StorageError, StorageErrorCode } from '../storage.error.js'; @@ -215,6 +217,39 @@ describe('StorageWorkspace', () => { }); }); + it('fails workspace writes closed when Files plugins cannot intercept conditional operations', async () => { + const transform = vi.fn(); + const driver = createFsStorageDriver({ + adapter: { root }, + plugins: [ + { + name: 'body-transform', + wrap: handlers({ + upload: (operation, next) => { + transform(operation.body); + return next({ ...operation, body: 'ciphertext' }); + }, + }), + }, + ], + }); + const workspace = mountStorageWorkspace( + new StorageClient('files-policy', driver), + { permissions: ALL_PERMISSIONS, prefix: 'runs/run-1' }, + ); + + await expect( + workspace.writeFile('plaintext.txt', 'plaintext', { mode: 'create' }), + ).rejects.toMatchObject({ + code: StorageErrorCode.NOT_SUPPORTED, + permanent: true, + }); + expect(transform).not.toHaveBeenCalled(); + await expect(driver.exists('runs/run-1/plaintext.txt')).resolves.toBe( + false, + ); + }); + it('rejects non-canonical ETags for every conditional mutation', async () => { const driver = createFsStorageDriver({ adapter: { root } }); const workspace = mountStorageWorkspace(