diff --git a/.changeset/storage-privacy-policy.md b/.changeset/storage-privacy-policy.md new file mode 100644 index 000000000..5afbab601 --- /dev/null +++ b/.changeset/storage-privacy-policy.md @@ -0,0 +1,9 @@ +--- +'oc': patch +'oc-azure-storage-adapter': patch +'oc-gs-storage-adapter': patch +'oc-s3-storage-adapter': patch +'oc-storage-adapters-utils': patch +--- + +Move OC component privacy classification out of the storage adapters. diff --git a/packages/oc-azure-storage-adapter/src/index.ts b/packages/oc-azure-storage-adapter/src/index.ts index e87c25a26..e37c4eb6e 100644 --- a/packages/oc-azure-storage-adapter/src/index.ts +++ b/packages/oc-azure-storage-adapter/src/index.ts @@ -13,6 +13,7 @@ import Cache from 'nice-cache'; import nodeDir, { type PathsResult } from 'node-dir'; import { getFileInfo, + type IsPrivateFile, type StorageAdapter, type StorageAdapterBaseConfig, strings @@ -183,7 +184,11 @@ export default function azureAdapter(conf: AzureConfig): StorageAdapter { return subDirectories; }; - const putDir = async (dirInput: string, dirOutput: string) => { + const putDir = async ( + dirInput: string, + dirOutput: string, + isPrivateFile: IsPrivateFile = () => false + ) => { const paths = await getPaths(dirInput); const packageJsonFile = path.join(dirInput, 'package.json'); const files = paths.files.filter((file) => file !== packageJsonFile); @@ -193,14 +198,7 @@ export default function azureAdapter(conf: AzureConfig): StorageAdapter { const relativeFile = file.slice(dirInput.length); const url = (dirOutput + relativeFile).replace(/\\/g, '/'); - const serverPattern = /(\\|\/)server\.js/; - const dotFilePattern = /(\\|\/)\..+/; - const privateFilePatterns = [serverPattern, dotFilePattern]; - return putFile( - file, - url, - privateFilePatterns.some((r) => r.test(relativeFile)) - ); + return putFile(file, url, isPrivateFile(relativeFile)); }) ); // Ensuring package.json is uploaded last so we can verify that a component @@ -208,7 +206,7 @@ export default function azureAdapter(conf: AzureConfig): StorageAdapter { const packageJsonFileResult = await putFile( packageJsonFile, `${dirOutput}/package.json`.replace(/\\/g, '/'), - false + isPrivateFile(packageJsonFile.slice(dirInput.length)) ); return [...filesResults, packageJsonFileResult]; diff --git a/packages/oc-azure-storage-adapter/test/privateFilesExclusion.test.ts b/packages/oc-azure-storage-adapter/test/privateFilesExclusion.test.ts index 1dc3b7053..d11b1f9f8 100644 --- a/packages/oc-azure-storage-adapter/test/privateFilesExclusion.test.ts +++ b/packages/oc-azure-storage-adapter/test/privateFilesExclusion.test.ts @@ -1,7 +1,7 @@ /* eslint-disable @typescript-eslint/no-non-null-assertion */ import adapter from '../src'; -test('put directory recognizes server.js and .env to be private', async () => { +test('put directory uses the supplied privacy classifier', async () => { const client = adapter({ publicContainerName: 'pubcon', privateContainerName: 'privcon', @@ -11,7 +11,9 @@ test('put directory recognizes server.js and .env to be private', async () => { componentsDir: 'components' }); - const mockResult = (await client.putDir('.', '.')) as Array<{ + const mockResult = (await client.putDir('.', '.', (filePath) => + filePath.endsWith('template.js') + )) as Array<{ fileName: string; container: string; }>; @@ -20,8 +22,8 @@ test('put directory recognizes server.js and .env to be private', async () => { const packageMock = mockResult.find((x) => x.fileName === './package.json')!; const templateMock = mockResult.find((x) => x.fileName === './template.js')!; - expect(serverMock.container).toBe('privcon'); - expect(envMock.container).toBe('privcon'); + expect(serverMock.container).toBe('pubcon'); + expect(envMock.container).toBe('pubcon'); expect(packageMock.container).toBe('pubcon'); - expect(templateMock.container).toBe('pubcon'); + expect(templateMock.container).toBe('privcon'); }); diff --git a/packages/oc-gs-storage-adapter/src/index.ts b/packages/oc-gs-storage-adapter/src/index.ts index 093cb4023..7370723ad 100644 --- a/packages/oc-gs-storage-adapter/src/index.ts +++ b/packages/oc-gs-storage-adapter/src/index.ts @@ -6,6 +6,7 @@ import Cache from 'nice-cache'; import nodeDir, { type PathsResult } from 'node-dir'; import { getFileInfo, + type IsPrivateFile, type StorageAdapter, type StorageAdapterBaseConfig, strings @@ -176,7 +177,11 @@ export default function gsAdapter(conf: GsConfig): StorageAdapter { } }; - const putDir = async (dirInput: string, dirOutput: string) => { + const putDir = async ( + dirInput: string, + dirOutput: string, + isPrivateFile: IsPrivateFile = () => false + ) => { const paths = await getPaths(dirInput); const packageJsonFile = path.join(dirInput, 'package.json'); const files = paths.files.filter((file) => file !== packageJsonFile); @@ -187,15 +192,7 @@ export default function gsAdapter(conf: GsConfig): StorageAdapter { const relativeFile = file.slice(dirInput.length); const url = (dirOutput + relativeFile).replace(/\\/g, '/'); - const serverPattern = /(\\|\/)server\.js/; - const dotFilePattern = /(\\|\/)\..+/; - const privateFilePatterns = [serverPattern, dotFilePattern]; - return putFile( - file, - url, - privateFilePatterns.some((r) => r.test(relativeFile)), - client - ); + return putFile(file, url, isPrivateFile(relativeFile), client); }) ); // Ensuring package.json is uploaded last so we can verify that a component @@ -203,7 +200,7 @@ export default function gsAdapter(conf: GsConfig): StorageAdapter { const packageJsonFileResult = await putFile( packageJsonFile, `${dirOutput}/package.json`.replace(/\\/g, '/'), - false, + isPrivateFile(packageJsonFile.slice(dirInput.length)), client ); diff --git a/packages/oc-gs-storage-adapter/test/privateFilesExclusion.test.ts b/packages/oc-gs-storage-adapter/test/privateFilesExclusion.test.ts index 44b41a64a..f1458dd95 100644 --- a/packages/oc-gs-storage-adapter/test/privateFilesExclusion.test.ts +++ b/packages/oc-gs-storage-adapter/test/privateFilesExclusion.test.ts @@ -20,7 +20,7 @@ jest.mock('node-dir', () => { }; }); -test('put directory recognizes server.js and .env to be private', async () => { +test('put directory uses the supplied privacy classifier', async () => { const options = { bucket: 'test', projectId: '12345', @@ -29,7 +29,9 @@ test('put directory recognizes server.js and .env to be private', async () => { }; const client = gs(options); - const mockResult = (await client.putDir('.', '.')) as Array<{ + const mockResult = (await client.putDir('.', '.', (filePath) => + filePath.endsWith('template.js') + )) as Array<{ Key: string; ACL: string; }>; @@ -38,8 +40,8 @@ test('put directory recognizes server.js and .env to be private', async () => { const packageMock = mockResult.find((x) => x.Key === './package.json')!; const templateMock = mockResult.find((x) => x.Key === './template.js')!; - expect(serverMock.ACL).toBe('authenticated-read'); - expect(envMock.ACL).toBe('authenticated-read'); + expect(serverMock.ACL).toBe('public-read'); + expect(envMock.ACL).toBe('public-read'); expect(packageMock.ACL).toBe('public-read'); - expect(templateMock.ACL).toBe('public-read'); + expect(templateMock.ACL).toBe('authenticated-read'); }); diff --git a/packages/oc-s3-storage-adapter/src/index.ts b/packages/oc-s3-storage-adapter/src/index.ts index 9447c67be..d97b7396e 100644 --- a/packages/oc-s3-storage-adapter/src/index.ts +++ b/packages/oc-s3-storage-adapter/src/index.ts @@ -14,6 +14,7 @@ import nodeDir, { type PathsResult } from 'node-dir'; import { getFileInfo, getNextYear, + type IsPrivateFile, type StorageAdapter, type StorageAdapterBaseConfig, strings @@ -235,7 +236,11 @@ export default function s3Adapter(conf: S3Config): StorageAdapter { return result; }; - const putDir = async (dirInput: string, dirOutput: string) => { + const putDir = async ( + dirInput: string, + dirOutput: string, + isPrivateFile: IsPrivateFile = () => false + ) => { const paths = await getPaths(dirInput); const packageJsonFile = path.join(dirInput, 'package.json'); const files = paths.files.filter((file) => file !== packageJsonFile); @@ -246,15 +251,7 @@ export default function s3Adapter(conf: S3Config): StorageAdapter { const relativeFile = file.slice(dirInput.length); const url = (dirOutput + relativeFile).replace(/\\/g, '/'); - const serverPattern = /(\\|\/)server\.js/; - const dotFilePattern = /(\\|\/)\..+/; - const privateFilePatterns = [serverPattern, dotFilePattern]; - return putFile( - file, - url, - privateFilePatterns.some((r) => r.test(relativeFile)), - client - ); + return putFile(file, url, isPrivateFile(relativeFile), client); }) ); // Ensuring package.json is uploaded last so we can verify that a component @@ -262,7 +259,7 @@ export default function s3Adapter(conf: S3Config): StorageAdapter { const packageJsonFileResult = await putFile( packageJsonFile, `${dirOutput}/package.json`.replace(/\\/g, '/'), - false, + isPrivateFile(packageJsonFile.slice(dirInput.length)), client ); diff --git a/packages/oc-s3-storage-adapter/test/privateFilesExclusion.test.ts b/packages/oc-s3-storage-adapter/test/privateFilesExclusion.test.ts index d4e8703df..eba3b7a31 100644 --- a/packages/oc-s3-storage-adapter/test/privateFilesExclusion.test.ts +++ b/packages/oc-s3-storage-adapter/test/privateFilesExclusion.test.ts @@ -1,7 +1,7 @@ /* eslint-disable @typescript-eslint/no-non-null-assertion */ import s3 from '../src'; -test('put directory recognizes server.js and .env to be private', async () => { +test('put directory uses the supplied privacy classifier', async () => { const options = { bucket: 'test', region: 'region-test', @@ -13,7 +13,9 @@ test('put directory recognizes server.js and .env to be private', async () => { const client = s3(options); - const mockResult = (await client.putDir('.', '.')) as Array<{ + const mockResult = (await client.putDir('.', '.', (filePath) => + filePath.endsWith('template.js') + )) as Array<{ Key: string; ACL: string; }>; @@ -22,8 +24,8 @@ test('put directory recognizes server.js and .env to be private', async () => { const packageMock = mockResult.find((x) => x.Key === './package.json')!; const templateMock = mockResult.find((x) => x.Key === './template.js')!; - expect(serverMock.ACL).toBe('authenticated-read'); - expect(envMock.ACL).toBe('authenticated-read'); + expect(serverMock.ACL).toBe('public-read'); + expect(envMock.ACL).toBe('public-read'); expect(packageMock.ACL).toBe('public-read'); - expect(templateMock.ACL).toBe('public-read'); + expect(templateMock.ACL).toBe('authenticated-read'); }); diff --git a/packages/oc-storage-adapters-utils/src/index.ts b/packages/oc-storage-adapters-utils/src/index.ts index c2d5cedae..a675a93ed 100644 --- a/packages/oc-storage-adapters-utils/src/index.ts +++ b/packages/oc-storage-adapters-utils/src/index.ts @@ -29,6 +29,9 @@ export interface StorageAdapterBaseConfig { refreshInterval?: number; } +/** Classifies a path relative to the directory passed to `putDir`. */ +export type IsPrivateFile = (filePath: string) => boolean; + export interface StorageAdapter { adapterType: string; getFile(filePath: string, force?: boolean): Promise; @@ -36,7 +39,11 @@ export interface StorageAdapter { getUrl: (componentName: string, version: string, fileName: string) => string; listSubDirectories(dir: string): Promise; maxConcurrentRequests: number; - putDir(folderPath: string, filePath: string): Promise; + putDir( + folderPath: string, + filePath: string, + isPrivateFile?: IsPrivateFile + ): Promise; putFile( filePath: string, fileName: string, diff --git a/packages/oc/src/registry/domain/repository.ts b/packages/oc/src/registry/domain/repository.ts index 2e03d357e..f31827ea3 100644 --- a/packages/oc/src/registry/domain/repository.ts +++ b/packages/oc/src/registry/domain/repository.ts @@ -28,7 +28,9 @@ import { reconcileMetadataFromStorage } from './metadata-migration'; import registerTemplates from './register-templates'; -import getPromiseBasedAdapter from './storage-adapter'; +import getPromiseBasedAdapter, { + isPrivateComponentFile +} from './storage-adapter'; import * as validator from './validators'; import * as versionHandler from './version-handler'; @@ -602,7 +604,8 @@ export default function repository(conf: Config) { try { await cdn.putDir( pkgDetails.outputFolder, - `${options!.componentsDir}/${componentName}/${componentVersion}` + `${options!.componentsDir}/${componentName}/${componentVersion}`, + isPrivateComponentFile ); await metadataStore.commitVersion( componentName, @@ -622,7 +625,8 @@ export default function repository(conf: Config) { await cdn.putDir( pkgDetails.outputFolder, - `${options!.componentsDir}/${componentName}/${componentVersion}` + `${options!.componentsDir}/${componentName}/${componentVersion}`, + isPrivateComponentFile ); invalidateComponentInfo(componentName, componentVersion); diff --git a/packages/oc/src/registry/domain/storage-adapter.ts b/packages/oc/src/registry/domain/storage-adapter.ts index 7897d9c8e..609346a1a 100644 --- a/packages/oc/src/registry/domain/storage-adapter.ts +++ b/packages/oc/src/registry/domain/storage-adapter.ts @@ -1,6 +1,12 @@ import type { StorageAdapter } from 'oc-storage-adapters-utils'; import { fromCallback } from 'universalify'; +const privateComponentFilePatterns = [/(\\|\/)server\.js/, /(\\|\/)\..+/]; + +export function isPrivateComponentFile(filePath: string): boolean { + return privateComponentFilePatterns.some((pattern) => pattern.test(filePath)); +} + type RemovePromiseOverload = T extends { (...args: infer B): void; (...args: any[]): Promise; @@ -48,11 +54,16 @@ function isLegacyAdapter( } function convertLegacyAdapter(adapter: LegacyStorageAdapter): StorageAdapter { + const putDir = fromCallback(adapter.putDir as any); + return { getFile: fromCallback(adapter.getFile as any), getJson: fromCallback(adapter.getJson as any), listSubDirectories: fromCallback(adapter.listSubDirectories as any), - putDir: fromCallback(adapter.putDir as any), + // Legacy callback adapters use their third argument for the callback and + // already own their directory privacy behaviour. + putDir: (folderPath: string, filePath: string) => + putDir(folderPath, filePath), putFile: fromCallback(adapter.putFile as any), putFileContent: fromCallback(adapter.putFileContent as any), getUrl: adapter.getUrl, diff --git a/packages/oc/test/unit/registry-domain-repository.js b/packages/oc/test/unit/registry-domain-repository.js index ca133e092..1c01c4f54 100644 --- a/packages/oc/test/unit/registry-domain-repository.js +++ b/packages/oc/test/unit/registry-domain-repository.js @@ -481,6 +481,10 @@ describe('registry : domain : repository', () => { expect(s3Mock.putDir.args[0][1]).to.equal( 'components/hello-world/1.0.1' ); + const isPrivateFile = s3Mock.putDir.args[0][2]; + expect(isPrivateFile('/server.js')).to.be.true; + expect(isPrivateFile('/.env')).to.be.true; + expect(isPrivateFile('/template.js')).to.be.false; }); }); diff --git a/packages/oc/test/unit/registry-domain-storage-adapter.js b/packages/oc/test/unit/registry-domain-storage-adapter.js index a6e2b549a..ac3cd7a80 100644 --- a/packages/oc/test/unit/registry-domain-storage-adapter.js +++ b/packages/oc/test/unit/registry-domain-storage-adapter.js @@ -32,18 +32,37 @@ function mockLegacyAdapter() { let process; -function initialise(adapter) { +function initialiseModule(fromCallback = sinon.stub().returns('promisified')) { process = { emitWarning: sinon.stub() }; - const adapterParser = injectr( + return injectr( '../../dist/registry/domain/storage-adapter.js', - { universalify: { fromCallback: sinon.stub().returns('promisified') } }, + { universalify: { fromCallback } }, { process } - ).default; + ); +} - return adapterParser(adapter); +function initialise(adapter) { + return initialiseModule().default(adapter); } describe('registry : domain : adapter', () => { + describe('component file privacy', () => { + const isPrivateComponentFile = + initialiseModule().isPrivateComponentFile; + + it('keeps server.js and dot-files private on any platform', () => { + expect(isPrivateComponentFile('/server.js')).to.be.true; + expect(isPrivateComponentFile('\\nested\\server.js')).to.be.true; + expect(isPrivateComponentFile('/.env')).to.be.true; + expect(isPrivateComponentFile('\\nested\\.secret')).to.be.true; + }); + + it('keeps other component files public', () => { + expect(isPrivateComponentFile('/package.json')).to.be.false; + expect(isPrivateComponentFile('/template.js')).to.be.false; + }); + }); + describe('when is not a legacy adapter', () => { const adapter = mockAdapter(); const parsed = initialise(adapter); @@ -89,9 +108,26 @@ describe('registry : domain : adapter', () => { expect(parsed.getFile).to.be.equal('promisified'); expect(parsed.getJson).to.be.equal('promisified'); expect(parsed.listSubDirectories).to.be.equal('promisified'); - expect(parsed.putDir).to.be.equal('promisified'); + expect(parsed.putDir).to.be.a('function'); expect(parsed.putFile).to.be.equal('promisified'); expect(parsed.putFileContent).to.be.equal('promisified'); }); + + it('keeps the optional classifier out of the legacy callback slot', async () => { + const adapter = mockLegacyAdapter(); + const classifier = sinon.stub().returns(true); + const fromCallback = (fn) => + (...args) => + new Promise((resolve, reject) => { + fn(...args, (err, result) => (err ? reject(err) : resolve(result))); + }); + const parsed = initialiseModule(fromCallback).default(adapter); + + await parsed.putDir('source', 'destination', classifier); + + expect(adapter.putDir.calledOnce).to.be.true; + expect(adapter.putDir.args[0]).to.have.length(3); + expect(adapter.putDir.args[0][2]).not.to.equal(classifier); + }); }); });