feat(simple-file-server): add SFTP storage support with extensible provider architecture#65
Merged
marioserrano09 merged 1 commit intoMay 19, 2026
Conversation
- Add StorageTarget type and SftpConfig interface to types - Create StorageProvider interface and StorageProviderRegistry for extensible storage backends - Extract LocalStorageProvider from StorageService - Add SftpStorageProvider using ssh2-sftp-client with connection pooling and local staging - Refactor StorageService as a provider-routing facade (registerProvider API) - Update BucketService.create() to accept storageTarget and sftpConfig - Move startup validation from BucketService to StorageService.validateStartup() - Update ThumbnailService to skip non-local buckets gracefully - Add SFTP error codes (SFTP_CONNECTION_FAILED, SFTP_AUTH_FAILED, SFTP_OPERATION_FAILED) - Update runtime to register SftpStorageProvider and close providers on shutdown - Update CLI create bucket command with --storage, --sftp-* options - Export new types and classes from public API - Add ssh2-sftp-client + @types/ssh2-sftp-client dependencies - Add 12 new tests covering registry, routing, SFTP bucket creation, and thumbnail skip Agent-Logs-Url: https://github.com/dynamiatools/framework/sessions/97161935-81bf-45e8-8ff8-2e03a9172fd6 Co-authored-by: marioserrano09 <5221275+marioserrano09@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
marioserrano09
May 19, 2026 07:36
View session
There was a problem hiding this comment.
Pull request overview
This PR extends extensions/entity-files/packages/simple-file-server with an extensible storage-provider architecture and adds an SFTP backend, allowing buckets to be backed by either local filesystem storage or a remote SFTP server.
Changes:
- Introduces
StorageProvider+StorageProviderRegistry, refactors storage logic intoLocalStorageProvider, and routes operations viaStorageService. - Adds
SftpStorageProvider(ssh2-sftp-client) and updates runtime/CLI/bucket metadata to supportstorageTarget: 'sftp'+sftpConfig. - Updates thumbnail behavior to be local-only and expands test coverage for provider routing and SFTP bucket creation.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| extensions/entity-files/packages/simple-file-server/test/core.test.ts | Adds tests for registry behavior, provider routing, SFTP bucket creation, and thumbnail skipping for SFTP. |
| extensions/entity-files/packages/simple-file-server/src/types/index.ts | Adds StorageTarget, SftpConfig, and extends Bucket metadata for storage backends. |
| extensions/entity-files/packages/simple-file-server/src/thumbnail/thumbnail.service.ts | Skips thumbnail generation for non-local buckets. |
| extensions/entity-files/packages/simple-file-server/src/storage/storage.service.ts | Refactors into a facade that dispatches operations to providers and adds startup validation/shutdown closing. |
| extensions/entity-files/packages/simple-file-server/src/storage/storage.provider.ts | Adds the provider interface and registry abstraction. |
| extensions/entity-files/packages/simple-file-server/src/storage/local.provider.ts | Extracts existing local FS storage behavior into a provider implementation. |
| extensions/entity-files/packages/simple-file-server/src/storage/sftp.provider.ts | Implements SFTP backend with per-bucket connections and local staging for uploads. |
| extensions/entity-files/packages/simple-file-server/src/storage/bucket.service.ts | Updates bucket creation to support SFTP buckets and persist sftpConfig. |
| extensions/entity-files/packages/simple-file-server/src/runtime/index.ts | Registers SFTP provider by default; delegates startup validation/shutdown closing to StorageService. |
| extensions/entity-files/packages/simple-file-server/src/index.ts | Exports new provider classes/types for SDK consumers. |
| extensions/entity-files/packages/simple-file-server/src/errors/index.ts | Adds SFTP-related error codes. |
| extensions/entity-files/packages/simple-file-server/src/cli/index.ts | Adds CLI flags to create SFTP-backed buckets and initializes them. |
| extensions/entity-files/packages/simple-file-server/package.json | Adds ssh2-sftp-client and types; updates dev deps/engines. |
| extensions/entity-files/packages/simple-file-server/package-lock.json | Lockfile updates for the new dependencies/tooling versions. |
Files not reviewed (1)
- extensions/entity-files/packages/simple-file-server/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)
extensions/entity-files/packages/simple-file-server/src/storage/bucket.service.ts:58
BucketService.create()only checks thatsftpConfigexists, but does not validate authentication fields. Given theSftpConfigdocs (“password mutually exclusive with privateKey”), consider enforcing: exactly one ofpasswordorprivateKeyis provided, and reject configurations that provide neither or both (and optionally rejectpassphrasewithoutprivateKey).
if (storageTarget === 'sftp') {
if (!sftpConfig) {
throw new SFSError(
SFSErrorCode.BUCKET_PATH_INVALID,
`sftpConfig is required when storageTarget is 'sftp'`,
400,
)
}
|
|
||
| export type Permission = 'read' | 'write' | 'delete' | ||
|
|
||
| export type StorageTarget = 'local' | 'sftp' |
Comment on lines
+51
to
+69
| if (storageTarget === 'sftp') { | ||
| if (!sftpConfig) { | ||
| throw new SFSError( | ||
| SFSErrorCode.BUCKET_PATH_INVALID, | ||
| `sftpConfig is required when storageTarget is 'sftp'`, | ||
| 400, | ||
| ) | ||
| } | ||
|
|
||
| const bucket: Bucket = { | ||
| name, | ||
| path: bucketPath, | ||
| createdAt: new Date().toISOString(), | ||
| storageTarget: 'sftp', | ||
| sftpConfig, | ||
| } | ||
|
|
||
| await writeJsonFile(this.bucketFilePath(name), bucket) | ||
| return bucket |
Comment on lines
+124
to
+129
| // Ensure remote base directory exists | ||
| const client = await this.getClient(bucket) | ||
| await client.mkdir(bucket.path, true).catch(() => { | ||
| // mkdir may fail if directory already exists on some servers — safe to ignore | ||
| }) | ||
| } |
Comment on lines
+217
to
+225
| async delete(bucket: Bucket, key: string): Promise<void> { | ||
| const remote = this.remotePath(bucket, key) | ||
| const client = await this.getClient(bucket) | ||
| try { | ||
| await client.delete(remote) | ||
| } catch { | ||
| throw notFound(`Object '${key}' in bucket '${bucket.name}'`, SFSErrorCode.OBJECT_NOT_FOUND) | ||
| } | ||
| } |
Comment on lines
+234
to
+235
| } catch { | ||
| throw notFound(`Directory '${keyPrefix}' in bucket '${bucket.name}'`, SFSErrorCode.OBJECT_NOT_FOUND) |
Comment on lines
+60
to
93
| const storageTarget = opts.storage as 'local' | 'sftp' | ||
| let bucket: Bucket | ||
|
|
||
| if (storageTarget === 'sftp') { | ||
| if (!opts.sftpHost) { | ||
| console.error(JSON.stringify({ ok: false, error: { code: 'SFS_INVALID_ARGS', message: '--sftp-host is required for sftp storage' } })) | ||
| process.exit(1) | ||
| } | ||
| if (!opts.sftpUsername) { | ||
| console.error(JSON.stringify({ ok: false, error: { code: 'SFS_INVALID_ARGS', message: '--sftp-username is required for sftp storage' } })) | ||
| process.exit(1) | ||
| } | ||
| if (!opts.sftpPassword && !opts.sftpPrivateKey) { | ||
| console.error(JSON.stringify({ ok: false, error: { code: 'SFS_INVALID_ARGS', message: 'Either --sftp-password or --sftp-private-key is required for sftp storage' } })) | ||
| process.exit(1) | ||
| } | ||
|
|
||
| bucket = await runtime.bucketService.create(name, bucketPath, 'sftp', { | ||
| host: opts.sftpHost as string, | ||
| port: parseInt(opts.sftpPort as string, 10), | ||
| username: opts.sftpUsername as string, | ||
| password: opts.sftpPassword as string | undefined, | ||
| privateKey: opts.sftpPrivateKey as string | undefined, | ||
| passphrase: opts.sftpPassphrase as string | undefined, | ||
| }) | ||
|
|
||
| // Initialise SFTP bucket (creates remote dir + local staging) | ||
| await runtime.storageService.initBucket(bucket) | ||
| } else { | ||
| bucket = await runtime.bucketService.create(name, bucketPath) | ||
| } | ||
|
|
||
| console.log(JSON.stringify({ ok: true, data: bucket }, null, 2)) | ||
| } catch (err: unknown) { |
Comment on lines
+75
to
+76
| for (const provider of this.registry.getAll().values()) { | ||
| await provider.close?.() |
Comment on lines
+64
to
+82
| const client = new SftpClient() | ||
| const connectOptions: SftpClient.ConnectOptions = { | ||
| host: config.host, | ||
| port: config.port ?? 22, | ||
| username: config.username, | ||
| } | ||
| if (config.password) connectOptions.password = config.password | ||
| if (config.privateKey) connectOptions.privateKey = config.privateKey | ||
| if (config.passphrase) connectOptions.passphrase = config.passphrase | ||
|
|
||
| try { | ||
| await client.connect(connectOptions) | ||
| } catch (err) { | ||
| throw new SFSError( | ||
| SFSErrorCode.SFTP_CONNECTION_FAILED, | ||
| `Failed to connect to SFTP server for bucket '${bucket.name}': ${err instanceof Error ? err.message : String(err)}`, | ||
| 503, | ||
| ) | ||
| } |
Comment on lines
+36
to
+72
| async create( | ||
| name: string, | ||
| bucketPath: string, | ||
| storageTarget: StorageTarget = 'local', | ||
| sftpConfig?: SftpConfig, | ||
| ): Promise<Bucket> { | ||
| if (!path.isAbsolute(bucketPath)) { | ||
| throw new SFSError(SFSErrorCode.BUCKET_PATH_INVALID, `Bucket path must be absolute: ${bucketPath}`, 400) | ||
| } | ||
|
|
||
| const existing = await this.find(name) | ||
| if (existing) { | ||
| throw conflict(`Bucket '${name}' already exists`, SFSErrorCode.BUCKET_ALREADY_EXISTS) | ||
| } | ||
|
|
||
| if (storageTarget === 'sftp') { | ||
| if (!sftpConfig) { | ||
| throw new SFSError( | ||
| SFSErrorCode.BUCKET_PATH_INVALID, | ||
| `sftpConfig is required when storageTarget is 'sftp'`, | ||
| 400, | ||
| ) | ||
| } | ||
|
|
||
| const bucket: Bucket = { | ||
| name, | ||
| path: bucketPath, | ||
| createdAt: new Date().toISOString(), | ||
| storageTarget: 'sftp', | ||
| sftpConfig, | ||
| } | ||
|
|
||
| await writeJsonFile(this.bucketFilePath(name), bucket) | ||
| return bucket | ||
| } | ||
|
|
||
| // ── local storage ────────────────────────────────────────────────────── |
Comment on lines
+48
to
+86
| private async getClient(bucket: Bucket): Promise<SftpClient> { | ||
| const config = this.requireSftpConfig(bucket) | ||
| const key = bucket.name | ||
|
|
||
| const existing = this.connections.get(key) | ||
| if (existing) { | ||
| // Perform a lightweight liveness check before reusing | ||
| try { | ||
| await existing.cwd() | ||
| return existing | ||
| } catch { | ||
| // Dead connection — fall through to reconnect | ||
| this.connections.delete(key) | ||
| } | ||
| } | ||
|
|
||
| const client = new SftpClient() | ||
| const connectOptions: SftpClient.ConnectOptions = { | ||
| host: config.host, | ||
| port: config.port ?? 22, | ||
| username: config.username, | ||
| } | ||
| if (config.password) connectOptions.password = config.password | ||
| if (config.privateKey) connectOptions.privateKey = config.privateKey | ||
| if (config.passphrase) connectOptions.passphrase = config.passphrase | ||
|
|
||
| try { | ||
| await client.connect(connectOptions) | ||
| } catch (err) { | ||
| throw new SFSError( | ||
| SFSErrorCode.SFTP_CONNECTION_FAILED, | ||
| `Failed to connect to SFTP server for bucket '${bucket.name}': ${err instanceof Error ? err.message : String(err)}`, | ||
| 503, | ||
| ) | ||
| } | ||
|
|
||
| this.connections.set(key, client) | ||
| return client | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds SFTP storage support to
extensions/entity-files/packages/simple-file-serverusing an extensible storage provider architecture. Buckets can now be created with astorageTargetof'sftp', and the design makes it straightforward to add more storage backends in the future.Changes
New abstractions
StorageProviderinterface (src/storage/storage.provider.ts) — defines the contract for any storage backend (initBucket,validateBucket,download,upload,delete,listDirectory,listBucket,close)StorageProviderRegistry— maps target names to provider instances; new backends can be registered at runtime withstorageService.registerProvider(target, provider)New providers
LocalStorageProvider(src/storage/local.provider.ts) — extracted from the originalStorageService, all local filesystem logic unchangedSftpStorageProvider(src/storage/sftp.provider.ts) — SFTP backend usingssh2-sftp-clientwith:Updated services
StorageService— refactored as a routing facade; auto-registersLocalStorageProvider; delegates all operations to the correct provider based onbucket.storageTarget; exposesvalidateStartup()andcloseProviders()BucketService.create()— accepts optionalstorageTargetandsftpConfig; skips local filesystem checks for SFTP bucketsThumbnailService— returnsnullfor non-'local'buckets (thumbnails remain local-only for now)runtime/index.ts— registersSftpStorageProviderautomatically; callsstorageService.validateStartup()andstorageService.closeProviders()on graceful shutdownUpdated types & errors
types/index.ts:StorageTarget,SftpConfig, updatedBucketerrors/index.ts:SFTP_CONNECTION_FAILED,SFTP_AUTH_FAILED,SFTP_OPERATION_FAILEDCLI
Adding future storage backends
Tests
StorageProviderRegistry(register, retrieve, overwrite, list)StorageServiceprovider routing (local fallback, custom provider dispatch)BucketServiceSFTP bucket creation, persistence, validationThumbnailServiceSFTP skip behaviour