From 6f365ffe73c25c7a900cbfc6d44c15f4095cc049 Mon Sep 17 00:00:00 2001 From: Aryan Jassal Date: Mon, 25 Nov 2024 16:24:09 +1100 Subject: [PATCH 1/6] feat: added vault efs resource aquisition --- src/vaults/Vault.ts | 2 + src/vaults/VaultInternal.ts | 83 ++++++++++++++++++++++++++++++++++++- 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/src/vaults/Vault.ts b/src/vaults/Vault.ts index 8c3981c6c..dbcf74bcd 100644 --- a/src/vaults/Vault.ts +++ b/src/vaults/Vault.ts @@ -8,6 +8,8 @@ interface Vault { writeG: VaultInternal['writeG']; readF: VaultInternal['readF']; readG: VaultInternal['readG']; + acquireRead: VaultInternal['acquireRead']; + acquireWrite: VaultInternal['acquireWrite']; log: VaultInternal['log']; version: VaultInternal['version']; } diff --git a/src/vaults/VaultInternal.ts b/src/vaults/VaultInternal.ts index 7d5a414c0..124b2cdfe 100644 --- a/src/vaults/VaultInternal.ts +++ b/src/vaults/VaultInternal.ts @@ -1,6 +1,8 @@ import type { ReadCommitResult } from 'isomorphic-git'; import type { EncryptedFS } from 'encryptedfs'; import type { DB, DBTransaction, LevelPath } from '@matrixai/db'; +import type { RPCClient } from '@matrixai/rpc'; +import type { ResourceAcquire, ResourceRelease } from '@matrixai/resources'; import type { CommitId, CommitLog, @@ -15,7 +17,6 @@ import type { import type KeyRing from '../keys/KeyRing'; import type { NodeId, NodeIdEncoded } from '../ids/types'; import type NodeManager from '../nodes/NodeManager'; -import type { RPCClient } from '@matrixai/rpc'; import type agentClientManifest from '../nodes/agent/callers'; import type { POJO } from '../types'; import path from 'path'; @@ -536,6 +537,86 @@ class VaultInternal { }); } + /** + * Acquire a read-only lock on this vault + */ + @ready(new vaultsErrors.ErrorVaultNotRunning()) + public acquireRead(): ResourceAcquire { + return async () => { + const acquire = this.lock.read(); + const [release] = await acquire(); + return [ + async (e?: Error) => { + await release(e); + }, + this.efsVault, + ]; + }; + } + + /** + * Acquire a read-write lock on this vault + */ + @ready(new vaultsErrors.ErrorVaultNotRunning()) + public acquireWrite( + tran?: DBTransaction, + ): ResourceAcquire { + return async () => { + let releaseTran: ResourceRelease | undefined = undefined; + const acquire = this.lock.write(); + const [release] = await acquire([tran]); + if (tran == null) { + const acquireTran = this.db.transaction(); + [releaseTran, tran] = await acquireTran(); + } + // The returned transaction can be undefined, too. We won't handle those + // cases. + if (tran == null) utils.never(); + await tran.lock( + [...this.vaultMetadataDbPath, VaultInternal.dirtyKey].join(''), + ); + if ( + (await tran.get([ + ...this.vaultMetadataDbPath, + VaultInternal.remoteKey, + ])) != null + ) { + // Mirrored vaults are immutable + throw new vaultsErrors.ErrorVaultRemoteDefined(); + } + await tran.put( + [...this.vaultMetadataDbPath, VaultInternal.dirtyKey], + true, + ); + return [ + async (e?: Error) => { + if (e != null) { + try { + // After doing mutation we need to commit the new history + await this.createCommit(); + } catch (e_) { + e = e_; + } + // This would happen if an error was caught inside the catch block + if (e != null) { + // Error implies dirty state + await this.cleanWorkingDirectory(); + } + } + // For some reason, the transaction type doesn't properly waterfall + // down to here. + await tran!.put( + [...this.vaultMetadataDbPath, VaultInternal.dirtyKey], + false, + ); + if (releaseTran != null) await releaseTran(e); + await release(e); + }, + this.efsVault, + ]; + }; + } + /** * Pulls changes to a vault from the vault's default remote. * If `pullNodeId` and `pullVaultNameOrId` it uses that for the remote instead. From 39ab49990ab41d69f55c6566e43f53d79df6586f Mon Sep 17 00:00:00 2001 From: Aryan Jassal Date: Tue, 26 Nov 2024 14:16:22 +1100 Subject: [PATCH 2/6] chore: added tests --- src/client/errors.ts | 6 + src/client/handlers/VaultsSecretsRemove.ts | 245 +++++++++++++++------ src/client/types.ts | 16 ++ tests/client/handlers/vaults.test.ts | 17 +- tests/vaults/VaultInternal.test.ts | 79 +++++++ 5 files changed, 290 insertions(+), 73 deletions(-) diff --git a/src/client/errors.ts b/src/client/errors.ts index 804a351d9..5db96fc8c 100644 --- a/src/client/errors.ts +++ b/src/client/errors.ts @@ -18,6 +18,11 @@ class ErrorClientAuthDenied extends ErrorClient { exitCode = sysexits.NOPERM; } +class ErrorClientInvalidHeader extends ErrorClient { + static description = 'The header message does not match the expected type' + exitCode = sysexits.USAGE; +} + class ErrorClientService extends ErrorClient {} class ErrorClientServiceRunning extends ErrorClientService { @@ -45,6 +50,7 @@ export { ErrorClientAuthMissing, ErrorClientAuthFormat, ErrorClientAuthDenied, + ErrorClientInvalidHeader, ErrorClientService, ErrorClientServiceRunning, ErrorClientServiceNotRunning, diff --git a/src/client/handlers/VaultsSecretsRemove.ts b/src/client/handlers/VaultsSecretsRemove.ts index 2f8a5cbba..1115f62e2 100644 --- a/src/client/handlers/VaultsSecretsRemove.ts +++ b/src/client/handlers/VaultsSecretsRemove.ts @@ -1,103 +1,204 @@ import type { DB } from '@matrixai/db'; +import { ResourceAcquire, withG } from '@matrixai/resources'; import type { ClientRPCRequestParams, ClientRPCResponseResult, - SecretIdentifierMessage, + SecretsRemoveHeaderMessage, + SecretIdentifierMessageTagged, SuccessOrErrorMessage, } from '../types'; import type VaultManager from '../../vaults/VaultManager'; import { DuplexHandler } from '@matrixai/rpc'; import * as vaultsUtils from '../../vaults/utils'; import * as vaultsErrors from '../../vaults/errors'; +import * as clientErrors from '../errors'; +import { FileSystemWritable } from '../../vaults/types'; +import * as utils from '../../utils'; class VaultsSecretsRemove extends DuplexHandler< { db: DB; vaultManager: VaultManager; }, - ClientRPCRequestParams, + ClientRPCRequestParams< + SecretsRemoveHeaderMessage | SecretIdentifierMessageTagged + >, ClientRPCResponseResult > { public handle = async function* ( - input: AsyncIterable>, + input: AsyncIterable< + ClientRPCRequestParams< + SecretsRemoveHeaderMessage | SecretIdentifierMessageTagged + > + >, ): AsyncGenerator> { const { db, vaultManager }: { db: DB; vaultManager: VaultManager } = this.container; - // Create a record of secrets to be removed, grouped by vault names - const vaultGroups: Record> = {}; - const secretNames: Array<[string, string]> = []; - let metadata: any = undefined; - for await (const secretRemoveMessage of input) { - if (metadata == null) metadata = secretRemoveMessage.metadata ?? {}; - secretNames.push([ - secretRemoveMessage.nameOrId, - secretRemoveMessage.secretName, - ]); - } - secretNames.forEach(([vaultName, secretName]) => { - if (vaultGroups[vaultName] == null) { - vaultGroups[vaultName] = []; + const vaultAcquires: Array> = []; + // Extracts the header message from the iterator + const headerMessage = await (async () => { + let header: SecretsRemoveHeaderMessage | undefined; + for await (const value of input) { + // TS cannot properly narrow down this type as it is too deeply wrapped. + // The as keyword is used to help it with type narrowing. + const message = value as + | SecretIdentifierMessageTagged + | SecretsRemoveHeaderMessage; + if (message.type === 'VaultNamesHeaderMesage') header = message; + break; } - vaultGroups[vaultName].push(secretName); - }); - // Now, all the paths will be removed for a vault within a single commit - yield* db.withTransactionG( - async function* (tran): AsyncGenerator { - for (const [vaultName, secretNames] of Object.entries(vaultGroups)) { - const vaultIdFromName = await vaultManager.getVaultId( - vaultName, - tran, + // The header message is mandatory + if (header == null) throw new clientErrors.ErrorClientInvalidHeader(); + return header; + })(); + // Create an array of write acquires + await db.withTransactionF(async (tran) => { + for (const vaultName of headerMessage!.vaultNames) { + const vaultIdFromName = await vaultManager.getVaultId(vaultName, tran); + const vaultId = vaultIdFromName ?? vaultsUtils.decodeVaultId(vaultName); + if (vaultId == null) { + throw new vaultsErrors.ErrorVaultsVaultUndefined( + `Vault ${vaultName} does not exist`, ); - const vaultId = - vaultIdFromName ?? vaultsUtils.decodeVaultId(vaultName); - if (vaultId == null) { - throw new vaultsErrors.ErrorVaultsVaultUndefined(); - } - yield* vaultManager.withVaultsG( - [vaultId], - async function* (vault): AsyncGenerator { - yield* vault.writeG( - async function* (efs): AsyncGenerator { - for (const secretName of secretNames) { - try { - const stat = await efs.stat(secretName); - if (stat.isDirectory()) { - await efs.rmdir(secretName, { - recursive: metadata?.options?.recursive, - }); - } else { - await efs.unlink(secretName); - } - yield { - type: 'success', - success: true, - }; - } catch (e) { - if ( - e.code === 'ENOENT' || - e.code === 'ENOTEMPTY' || - e.code === 'EINVAL' - ) { - // INVAL can be triggered if removing the root of the - // vault is attempted. - yield { - type: 'error', - code: e.code, - reason: secretName, - }; - } else { - throw e; - } - } - } - }, + } + await vaultManager.withVaults([vaultId], async (vault) => { + vaultAcquires.push(vault.acquireWrite()); + }); + } + }); + // Acquire all locks in parallel and perform all operations at once + yield* withG( + vaultAcquires, + async function* (efses): AsyncGenerator { + // Creating the vault name to efs map for easy access + const vaultMap = new Map(); + for (let i = 0; i < efses.length; i++) { + vaultMap.set(headerMessage!.vaultNames[i], efses[i]); + } + for await (const value of input) { + // TS cannot properly narrow down this type as it is too deeply wrapped. + // The as keyword is used to help it with type narrowing. + const message = value as + | SecretIdentifierMessageTagged + | SecretsRemoveHeaderMessage; + // Ignoring any header messages + if (message.type === 'SecretIdentifierMessage') { + const efs = vaultMap.get(message.nameOrId); + if (efs == null) { + throw new vaultsErrors.ErrorVaultsVaultUndefined( + `Vault ${message.nameOrId} was not present in the header message`, ); - }, - tran, - ); + } + try { + const stat = await efs.stat(message.secretName); + if (stat.isDirectory()) { + await efs.rmdir(message.secretName, { + recursive: headerMessage.recursive, + }); + } else { + await efs.unlink(message.secretName); + } + yield { + type: 'success', + success: true, + }; + } catch (e) { + if ( + e.code === 'ENOENT' || + e.code === 'ENOTEMPTY' || + e.code === 'EINVAL' + ) { + // INVAL can be triggered if removing the root of the + // vault is attempted. + yield { + type: 'error', + code: e.code, + reason: message.secretName, + }; + } else { + throw e; + } + } + } } }, ); + + // Create a record of secrets to be removed, grouped by vault names + // const vaultGroups: Record> = {}; + // const secretNames: Array<[string, string]> = []; + // let metadata: any = undefined; + // for await (const secretRemoveMessage of input) { + // if (metadata == null) metadata = secretRemoveMessage.metadata ?? {}; + // secretNames.push([ + // secretRemoveMessage.nameOrId, + // secretRemoveMessage.secretName, + // ]); + // } + // secretNames.forEach(([vaultName, secretName]) => { + // if (vaultGroups[vaultName] == null) { + // vaultGroups[vaultName] = []; + // } + // vaultGroups[vaultName].push(secretName); + // }); + // Now, all the paths will be removed for a vault within a single commit + // yield* db.withTransactionG( + // async function* (tran): AsyncGenerator { + // for (const [vaultName, secretNames] of Object.entries(vaultGroups)) { + // const vaultIdFromName = await vaultManager.getVaultId( + // vaultName, + // tran, + // ); + // const vaultId = + // vaultIdFromName ?? vaultsUtils.decodeVaultId(vaultName); + // if (vaultId == null) { + // throw new vaultsErrors.ErrorVaultsVaultUndefined(); + // } + // yield* vaultManager.withVaultsG( + // [vaultId], + // async function* (vault): AsyncGenerator { + // yield* vault.writeG( + // async function* (efs): AsyncGenerator { + // for (const secretName of secretNames) { + // try { + // const stat = await efs.stat(secretName); + // if (stat.isDirectory()) { + // await efs.rmdir(secretName, { + // recursive: metadata?.options?.recursive, + // }); + // } else { + // await efs.unlink(secretName); + // } + // yield { + // type: 'success', + // success: true, + // }; + // } catch (e) { + // if ( + // e.code === 'ENOENT' || + // e.code === 'ENOTEMPTY' || + // e.code === 'EINVAL' + // ) { + // // INVAL can be triggered if removing the root of the + // // vault is attempted. + // yield { + // type: 'error', + // code: e.code, + // reason: secretName, + // }; + // } else { + // throw e; + // } + // } + // } + // }, + // ); + // }, + // tran, + // ); + // } + // }, + // ); }; } diff --git a/src/client/types.ts b/src/client/types.ts index ae415bb1e..de48d9c83 100644 --- a/src/client/types.ts +++ b/src/client/types.ts @@ -360,6 +360,19 @@ type SecretStatMessage = { }; }; +type SecretIdentifierMessageTagged = SecretIdentifierMessage & { + type: 'SecretIdentifierMessage'; +} + +type VaultNamesHeaderMesage = { + type: 'VaultNamesHeaderMesage'; + vaultNames: Array; +}; + +type SecretsRemoveHeaderMessage = VaultNamesHeaderMesage & { + recursive?: boolean; +}; + // Type casting for tricky handlers type OverrideRPClientType> = Omit< @@ -435,6 +448,9 @@ export type { SecretRenameMessage, SecretFilesMessage, SecretStatMessage, + SecretIdentifierMessageTagged, + VaultNamesHeaderMesage, + SecretsRemoveHeaderMessage, SignatureMessage, OverrideRPClientType, AuditMetricGetTypeOverride, diff --git a/tests/client/handlers/vaults.test.ts b/tests/client/handlers/vaults.test.ts index 1f1b85713..9889cfc2f 100644 --- a/tests/client/handlers/vaults.test.ts +++ b/tests/client/handlers/vaults.test.ts @@ -3,10 +3,13 @@ import type { FileSystem } from '@/types'; import type { VaultId } from '@/ids'; import type NodeManager from '@/nodes/NodeManager'; import type { + ClientRPCRequestParams, ContentSuccessMessage, ErrorMessage, LogEntryMessage, SecretContentMessage, + SecretIdentifierMessage, + SecretsRemoveHeaderMessage, VaultListMessage, VaultPermissionMessage, } from '@/client/types'; @@ -2371,7 +2374,19 @@ describe('vaultsSecretsRemove', () => { // Write paths const response = await rpcClient.methods.vaultsSecretsRemove(); const writer = response.writable.getWriter(); - await writer.write({ nameOrId: 'invalid', secretName: 'invalid' }); + let variable: ClientRPCRequestParams = {type: 'VaultNamesHeaderMesage', vaultNames: ['invalid']}; + console.log(variable); + // Header message + await writer.write({ + type: 'VaultNamesHeaderMesage', + vaultNames: ['invalid'], + }); + // Content messages + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: 'invalid', + secretName: 'invalid', + }); await writer.close(); // Read response const consumeP = async () => { diff --git a/tests/vaults/VaultInternal.test.ts b/tests/vaults/VaultInternal.test.ts index a4d7ca88b..2d487da30 100644 --- a/tests/vaults/VaultInternal.test.ts +++ b/tests/vaults/VaultInternal.test.ts @@ -7,6 +7,7 @@ import os from 'os'; import path from 'path'; import fs from 'fs'; import { DB } from '@matrixai/db'; +import { withF } from '@matrixai/resources'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { EncryptedFS } from 'encryptedfs'; import git from 'isomorphic-git'; @@ -491,6 +492,8 @@ describe('VaultInternal', () => { expect(vaultInterface.writeG).toBeTruthy(); expect(vaultInterface.readF).toBeTruthy(); expect(vaultInterface.readG).toBeTruthy(); + expect(vaultInterface.acquireRead).toBeTruthy(); + expect(vaultInterface.acquireWrite).toBeTruthy(); expect(vaultInterface.log).toBeTruthy(); expect(vaultInterface.version).toBeTruthy(); @@ -873,6 +876,82 @@ describe('VaultInternal', () => { expect(finished.length).toBe(4); await releaseRead(); }); + test('can acquire a read resource', async () => { + await vault.writeF(async (efs) => { + await efs.writeFile(secret1.name, secret1.content); + }); + const acquireRead = vault.acquireRead(); + await withF([acquireRead], async ([efs]) => { + const content = await efs.readFile(secret1.name); + expect(content.toString()).toEqual(secret1.content); + }); + }); + test('acquiring read resource respects locking', async () => { + const lock = vault.getLock(); + const [releaseWrite] = await lock.write()(); + let finished = false; + const readP = withF([vault.acquireRead()], async () => { + finished = true; + }); + await sleep(waitDelay); + expect(finished).toBe(false); + await releaseWrite(); + await readP; + expect(finished).toBe(true); + }); + test('acquiring read resource allows concurrency', async () => { + const lock = vault.getLock(); + const [releaseRead] = await lock.read()(); + const finished: Array = []; + const read1P = withF([vault.acquireRead()], async () => { + finished.push(true); + }); + const read2P = withF([vault.acquireRead()], async () => { + finished.push(true); + }); + const read3P = withF([vault.acquireRead()], async () => { + finished.push(true); + }); + await Promise.all([read1P, read2P, read3P]); + expect(finished.length).toBe(3); + await releaseRead(); + }); + test('can acquire a write resource', async () => { + const acquireWrite = vault.acquireWrite(); + await withF([acquireWrite], async ([efs]) => { + await efs.writeFile(secret1.name, secret1.content); + }); + await vault.readF(async (efs) => { + const content = await efs.readFile(secret1.name); + expect(content.toString()).toEqual(secret1.content); + }); + }); + test('acquiring write resource respects write locking', async () => { + const lock = vault.getLock(); + const [releaseWrite] = await lock.write()(); + let finished = false; + const writeP = withF([vault.acquireWrite()], async () => { + finished = true; + }); + await sleep(waitDelay); + expect(finished).toBe(false); + await releaseWrite(); + await writeP; + expect(finished).toBe(true); + }); + test('acquiring write resource respects read locking', async () => { + const lock = vault.getLock(); + const [releaseRead] = await lock.read()(); + let finished = false; + const writeP = withF([vault.acquireWrite()], async () => { + finished = true; + }); + await sleep(waitDelay); + expect(finished).toBe(false); + await releaseRead(); + await writeP; + expect(finished).toBe(true); + }); // Life-cycle test('can create with CreateVaultInternal', async () => { let vault1: VaultInternal | undefined; From 01af272ca5b5426e577d01dd4e7b39d4db5c45be Mon Sep 17 00:00:00 2001 From: Aryan Jassal Date: Wed, 27 Nov 2024 18:22:55 +1100 Subject: [PATCH 3/6] chore: updated `VaultsSecretsRemove` to use new resource aquisition --- src/client/errors.ts | 2 +- src/client/handlers/VaultsSecretsRemove.ts | 181 +++++------------ src/client/types.ts | 23 ++- src/vaults/VaultInternal.ts | 4 +- tests/client/handlers/vaults.test.ts | 215 ++++++++++++++------- 5 files changed, 207 insertions(+), 218 deletions(-) diff --git a/src/client/errors.ts b/src/client/errors.ts index 5db96fc8c..7d5ca751a 100644 --- a/src/client/errors.ts +++ b/src/client/errors.ts @@ -19,7 +19,7 @@ class ErrorClientAuthDenied extends ErrorClient { } class ErrorClientInvalidHeader extends ErrorClient { - static description = 'The header message does not match the expected type' + static description = 'The header message does not match the expected type'; exitCode = sysexits.USAGE; } diff --git a/src/client/handlers/VaultsSecretsRemove.ts b/src/client/handlers/VaultsSecretsRemove.ts index 1115f62e2..bf57ba273 100644 --- a/src/client/handlers/VaultsSecretsRemove.ts +++ b/src/client/handlers/VaultsSecretsRemove.ts @@ -1,5 +1,5 @@ import type { DB } from '@matrixai/db'; -import { ResourceAcquire, withG } from '@matrixai/resources'; +import type { ResourceAcquire } from '@matrixai/resources'; import type { ClientRPCRequestParams, ClientRPCResponseResult, @@ -8,12 +8,12 @@ import type { SuccessOrErrorMessage, } from '../types'; import type VaultManager from '../../vaults/VaultManager'; +import type { FileSystemWritable } from '../../vaults/types'; +import { withG } from '@matrixai/resources'; import { DuplexHandler } from '@matrixai/rpc'; import * as vaultsUtils from '../../vaults/utils'; import * as vaultsErrors from '../../vaults/errors'; import * as clientErrors from '../errors'; -import { FileSystemWritable } from '../../vaults/types'; -import * as utils from '../../utils'; class VaultsSecretsRemove extends DuplexHandler< { @@ -37,23 +37,16 @@ class VaultsSecretsRemove extends DuplexHandler< const vaultAcquires: Array> = []; // Extracts the header message from the iterator const headerMessage = await (async () => { - let header: SecretsRemoveHeaderMessage | undefined; - for await (const value of input) { - // TS cannot properly narrow down this type as it is too deeply wrapped. - // The as keyword is used to help it with type narrowing. - const message = value as - | SecretIdentifierMessageTagged - | SecretsRemoveHeaderMessage; - if (message.type === 'VaultNamesHeaderMesage') header = message; - break; + const iterator = input[Symbol.asyncIterator](); + const header = (await iterator.next()).value; + if (header.type === 'VaultNamesHeaderMessage') { + if (header == null) throw new clientErrors.ErrorClientInvalidHeader(); + return header; } - // The header message is mandatory - if (header == null) throw new clientErrors.ErrorClientInvalidHeader(); - return header; })(); // Create an array of write acquires await db.withTransactionF(async (tran) => { - for (const vaultName of headerMessage!.vaultNames) { + for (const vaultName of headerMessage.vaultNames) { const vaultIdFromName = await vaultManager.getVaultId(vaultName, tran); const vaultId = vaultIdFromName ?? vaultsUtils.decodeVaultId(vaultName); if (vaultId == null) { @@ -61,9 +54,11 @@ class VaultsSecretsRemove extends DuplexHandler< `Vault ${vaultName} does not exist`, ); } - await vaultManager.withVaults([vaultId], async (vault) => { - vaultAcquires.push(vault.acquireWrite()); - }); + const acquire = await vaultManager.withVaults( + [vaultId], + async (vault) => vault.acquireWrite(), + ); + vaultAcquires.push(acquire); } }); // Acquire all locks in parallel and perform all operations at once @@ -75,130 +70,48 @@ class VaultsSecretsRemove extends DuplexHandler< for (let i = 0; i < efses.length; i++) { vaultMap.set(headerMessage!.vaultNames[i], efses[i]); } - for await (const value of input) { - // TS cannot properly narrow down this type as it is too deeply wrapped. - // The as keyword is used to help it with type narrowing. - const message = value as - | SecretIdentifierMessageTagged - | SecretsRemoveHeaderMessage; + for await (const message of input) { // Ignoring any header messages - if (message.type === 'SecretIdentifierMessage') { - const efs = vaultMap.get(message.nameOrId); - if (efs == null) { - throw new vaultsErrors.ErrorVaultsVaultUndefined( - `Vault ${message.nameOrId} was not present in the header message`, - ); + if (message.type !== 'SecretIdentifierMessage') continue; + const efs = vaultMap.get(message.nameOrId); + if (efs == null) { + throw new vaultsErrors.ErrorVaultsVaultUndefined( + `Vault ${message.nameOrId} was not present in the header message`, + ); + } + try { + const stat = await efs.stat(message.secretName); + if (stat.isDirectory()) { + await efs.rmdir(message.secretName, { + recursive: headerMessage.recursive, + }); + } else { + await efs.unlink(message.secretName); } - try { - const stat = await efs.stat(message.secretName); - if (stat.isDirectory()) { - await efs.rmdir(message.secretName, { - recursive: headerMessage.recursive, - }); - } else { - await efs.unlink(message.secretName); - } + yield { + type: 'success', + success: true, + }; + } catch (e) { + if ( + e.code === 'ENOENT' || + e.code === 'ENOTEMPTY' || + e.code === 'EINVAL' + ) { + // INVAL can be triggered if removing the root of the + // vault is attempted. yield { - type: 'success', - success: true, + type: 'error', + code: e.code, + reason: message.secretName, }; - } catch (e) { - if ( - e.code === 'ENOENT' || - e.code === 'ENOTEMPTY' || - e.code === 'EINVAL' - ) { - // INVAL can be triggered if removing the root of the - // vault is attempted. - yield { - type: 'error', - code: e.code, - reason: message.secretName, - }; - } else { - throw e; - } + } else { + throw e; } } } }, ); - - // Create a record of secrets to be removed, grouped by vault names - // const vaultGroups: Record> = {}; - // const secretNames: Array<[string, string]> = []; - // let metadata: any = undefined; - // for await (const secretRemoveMessage of input) { - // if (metadata == null) metadata = secretRemoveMessage.metadata ?? {}; - // secretNames.push([ - // secretRemoveMessage.nameOrId, - // secretRemoveMessage.secretName, - // ]); - // } - // secretNames.forEach(([vaultName, secretName]) => { - // if (vaultGroups[vaultName] == null) { - // vaultGroups[vaultName] = []; - // } - // vaultGroups[vaultName].push(secretName); - // }); - // Now, all the paths will be removed for a vault within a single commit - // yield* db.withTransactionG( - // async function* (tran): AsyncGenerator { - // for (const [vaultName, secretNames] of Object.entries(vaultGroups)) { - // const vaultIdFromName = await vaultManager.getVaultId( - // vaultName, - // tran, - // ); - // const vaultId = - // vaultIdFromName ?? vaultsUtils.decodeVaultId(vaultName); - // if (vaultId == null) { - // throw new vaultsErrors.ErrorVaultsVaultUndefined(); - // } - // yield* vaultManager.withVaultsG( - // [vaultId], - // async function* (vault): AsyncGenerator { - // yield* vault.writeG( - // async function* (efs): AsyncGenerator { - // for (const secretName of secretNames) { - // try { - // const stat = await efs.stat(secretName); - // if (stat.isDirectory()) { - // await efs.rmdir(secretName, { - // recursive: metadata?.options?.recursive, - // }); - // } else { - // await efs.unlink(secretName); - // } - // yield { - // type: 'success', - // success: true, - // }; - // } catch (e) { - // if ( - // e.code === 'ENOENT' || - // e.code === 'ENOTEMPTY' || - // e.code === 'EINVAL' - // ) { - // // INVAL can be triggered if removing the root of the - // // vault is attempted. - // yield { - // type: 'error', - // code: e.code, - // reason: secretName, - // }; - // } else { - // throw e; - // } - // } - // } - // }, - // ); - // }, - // tran, - // ); - // } - // }, - // ); }; } diff --git a/src/client/types.ts b/src/client/types.ts index de48d9c83..73d6ae271 100644 --- a/src/client/types.ts +++ b/src/client/types.ts @@ -1,7 +1,8 @@ import type { ClientManifest, JSONObject, - JSONRPCResponseResult, + JSONRPCResponseMetadata, + // JSONRPCResponseResult, RPCClient, } from '@matrixai/rpc'; import type { @@ -25,6 +26,16 @@ import type { } from '../nodes/types'; import type { AuditEventsGetTypeOverride } from './callers/auditEventsGet'; +// TEMP: For testing and debugging. This will go into js-rpc or something. +type JSONRPCResponseResult< + T extends JSONObject = JSONObject, + M extends JSONObject = JSONObject, +> = T & { + metadata?: JSONRPCResponseMetadata & + M & + (T extends { metadata: infer U } ? U : JSONObject); +}; + type ClientRPCRequestParams = JSONRPCResponseResult< T, @@ -362,14 +373,14 @@ type SecretStatMessage = { type SecretIdentifierMessageTagged = SecretIdentifierMessage & { type: 'SecretIdentifierMessage'; -} +}; -type VaultNamesHeaderMesage = { - type: 'VaultNamesHeaderMesage'; +type VaultNamesHeaderMessage = { + type: 'VaultNamesHeaderMessage'; vaultNames: Array; }; -type SecretsRemoveHeaderMessage = VaultNamesHeaderMesage & { +type SecretsRemoveHeaderMessage = VaultNamesHeaderMessage & { recursive?: boolean; }; @@ -449,7 +460,7 @@ export type { SecretFilesMessage, SecretStatMessage, SecretIdentifierMessageTagged, - VaultNamesHeaderMesage, + VaultNamesHeaderMessage, SecretsRemoveHeaderMessage, SignatureMessage, OverrideRPClientType, diff --git a/src/vaults/VaultInternal.ts b/src/vaults/VaultInternal.ts index 124b2cdfe..b896d3ce1 100644 --- a/src/vaults/VaultInternal.ts +++ b/src/vaults/VaultInternal.ts @@ -564,7 +564,7 @@ class VaultInternal { return async () => { let releaseTran: ResourceRelease | undefined = undefined; const acquire = this.lock.write(); - const [release] = await acquire([tran]); + const [release] = await acquire(); if (tran == null) { const acquireTran = this.db.transaction(); [releaseTran, tran] = await acquireTran(); @@ -590,7 +590,7 @@ class VaultInternal { ); return [ async (e?: Error) => { - if (e != null) { + if (e == null) { try { // After doing mutation we need to commit the new history await this.createCommit(); diff --git a/tests/client/handlers/vaults.test.ts b/tests/client/handlers/vaults.test.ts index 9889cfc2f..57b256cb3 100644 --- a/tests/client/handlers/vaults.test.ts +++ b/tests/client/handlers/vaults.test.ts @@ -3,13 +3,8 @@ import type { FileSystem } from '@/types'; import type { VaultId } from '@/ids'; import type NodeManager from '@/nodes/NodeManager'; import type { - ClientRPCRequestParams, - ContentSuccessMessage, - ErrorMessage, LogEntryMessage, SecretContentMessage, - SecretIdentifierMessage, - SecretsRemoveHeaderMessage, VaultListMessage, VaultPermissionMessage, } from '@/client/types'; @@ -76,6 +71,7 @@ import * as nodesUtils from '@/nodes/utils'; import * as vaultsUtils from '@/vaults/utils'; import * as vaultsErrors from '@/vaults/errors'; import * as networkUtils from '@/network/utils'; +import * as utils from '@/utils'; import * as testsUtils from '../../utils'; describe('vaultsClone', () => { @@ -1486,11 +1482,9 @@ describe('vaultsSecretsMkdir', () => { await writer.close(); for await (const data of response.readable) { expect(data.type).toEqual('error'); - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'error'. - const error = data as ErrorMessage; - expect(error.code).toEqual('ENOENT'); - expect(error.reason).toEqual(dirPath); + if (data.type !== 'error') utils.never(); + expect(data.code).toEqual('ENOENT'); + expect(data.reason).toEqual(dirPath); } await vaultManager.withVaults([vaultId], async (vault) => { await vault.readF(async (efs) => { @@ -1556,11 +1550,8 @@ describe('vaultsSecretsMkdir', () => { // Check if the operation concluded as expected for await (const data of response.readable) { if (data.type === 'error') { - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'error'. - const error = data as ErrorMessage; - expect(error.code).toEqual('ENOENT'); - expect(error.reason).toEqual(dirPath3); + expect(data.code).toEqual('ENOENT'); + expect(data.reason).toEqual(dirPath3); } } await vaultManager.withVaults( @@ -1594,11 +1585,9 @@ describe('vaultsSecretsMkdir', () => { // Check if the operation concluded as expected for await (const data of response.readable) { expect(data.type).toEqual('error'); - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'error'. - const error = data as ErrorMessage; - expect(error.code).toEqual('EEXIST'); - expect(error.reason).toEqual(dirPath); + if (data.type !== 'error') utils.never(); + expect(data.code).toEqual('EEXIST'); + expect(data.reason).toEqual(dirPath); } await vaultManager.withVaults([vaultId], async (vault) => { await vault.readF(async (efs) => { @@ -1739,10 +1728,8 @@ describe('vaultsSecretsCat', () => { // Read response for await (const data of response.readable) { expect(data.type).toEqual('success'); - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'success'. - const message = data as ContentSuccessMessage; - expect(message.secretContent).toEqual(secretContent); + if (data.type !== 'success') utils.never(); + expect(data.secretContent).toEqual(secretContent); } }); test('fails to read invalid secret', async () => { @@ -1760,11 +1747,9 @@ describe('vaultsSecretsCat', () => { // Read response for await (const data of response.readable) { expect(data.type).toEqual('error'); - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'success'. - const error = data as ErrorMessage; - expect(error.code).toEqual('ENOENT'); - expect(error.reason).toEqual(secretName); + if (data.type !== 'error') utils.never(); + expect(data.code).toEqual('ENOENT'); + expect(data.reason).toEqual(secretName); } }); test('fails to read a directory', async () => { @@ -1788,11 +1773,9 @@ describe('vaultsSecretsCat', () => { // Read response for await (const data of response.readable) { expect(data.type).toEqual('error'); - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'success'. - const error = data as ErrorMessage; - expect(error.code).toEqual('EISDIR'); - expect(error.reason).toEqual(secretName); + if (data.type !== 'error') utils.never(); + expect(data.code).toEqual('EISDIR'); + expect(data.reason).toEqual(secretName); } }); test('reads multiple secrets in order', async () => { @@ -1820,10 +1803,8 @@ describe('vaultsSecretsCat', () => { let totalContent = ''; for await (const data of response.readable) { expect(data.type).toEqual('success'); - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'success'. - const message = data as ContentSuccessMessage; - totalContent += message.secretContent; + if (data.type !== 'success') utils.never(); + totalContent += data.secretContent; } expect(totalContent).toEqual(`${secretContent1}${secretContent2}`); }); @@ -1864,10 +1845,8 @@ describe('vaultsSecretsCat', () => { let totalContent = ''; for await (const data of response.readable) { expect(data.type).toEqual('success'); - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'success'. - const message = data as ContentSuccessMessage; - totalContent += message.secretContent; + if (data.type !== 'success') utils.never(); + totalContent += data.secretContent; } expect(totalContent).toEqual( `${secretContent1}${secretContent2}${secretContent3}`, @@ -1913,16 +1892,10 @@ describe('vaultsSecretsCat', () => { let totalContent = ''; for await (const data of response.readable) { if (data.type === 'success') { - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'success'. - const message = data as ContentSuccessMessage; - totalContent += message.secretContent; + totalContent += data.secretContent; } else { - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'success'. - const error = data as ErrorMessage; - expect(error.code).toEqual('ENOENT'); - expect(error.reason).toEqual(invalidName); + expect(data.code).toEqual('ENOENT'); + expect(data.reason).toEqual(invalidName); } } expect(totalContent).toEqual( @@ -2374,11 +2347,9 @@ describe('vaultsSecretsRemove', () => { // Write paths const response = await rpcClient.methods.vaultsSecretsRemove(); const writer = response.writable.getWriter(); - let variable: ClientRPCRequestParams = {type: 'VaultNamesHeaderMesage', vaultNames: ['invalid']}; - console.log(variable); // Header message await writer.write({ - type: 'VaultNamesHeaderMesage', + type: 'VaultNamesHeaderMessage', vaultNames: ['invalid'], }); // Content messages @@ -2410,17 +2381,27 @@ describe('vaultsSecretsRemove', () => { // Delete secrets const response = await rpcClient.methods.vaultsSecretsRemove(); const writer = response.writable.getWriter(); - await writer.write({ nameOrId: vaultIdEncoded, secretName: '/' }); + // Header message + await writer.write({ + type: 'VaultNamesHeaderMessage', + vaultNames: [vaultIdEncoded], + }); + // Content messages + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded, + secretName: '/', + }); await writer.close(); + let loopRun = false; for await (const data of response.readable) { + loopRun = true; expect(data.type).toStrictEqual('error'); - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'error'. - const error = data as ErrorMessage; - // The error code should be an invalid operation - expect(error.code).toStrictEqual('EINVAL'); + if (data.type !== 'error') utils.never(); + expect(data.code).toStrictEqual('EINVAL'); } // Check + expect(loopRun).toBeTruthy(); await vaultManager.withVaults([vaultId], async (vault) => { await vault.readF(async (efs) => { expect(await efs.exists(secretName)).toBeTruthy(); @@ -2442,12 +2423,29 @@ describe('vaultsSecretsRemove', () => { // Delete secrets const response = await rpcClient.methods.vaultsSecretsRemove(); const writer = response.writable.getWriter(); - await writer.write({ nameOrId: vaultIdEncoded, secretName: secretName1 }); - await writer.write({ nameOrId: vaultIdEncoded, secretName: secretName2 }); + // Header message + await writer.write({ + type: 'VaultNamesHeaderMessage', + vaultNames: [vaultIdEncoded], + }); + // Content messages + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded, + secretName: secretName1, + }); + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded, + secretName: secretName2, + }); await writer.close(); + let loopRun = false; for await (const data of response.readable) { + loopRun = true; expect(data.type).toStrictEqual('success'); } + expect(loopRun).toBeTruthy(); // Check each secret was deleted await vaultManager.withVaults([vaultId], async (vault) => { await vault.readF(async (efs) => { @@ -2472,18 +2470,33 @@ describe('vaultsSecretsRemove', () => { // Delete secrets const response = await rpcClient.methods.vaultsSecretsRemove(); const writer = response.writable.getWriter(); - await writer.write({ nameOrId: vaultIdEncoded, secretName: secretName1 }); - await writer.write({ nameOrId: vaultIdEncoded, secretName: invalidName }); - await writer.write({ nameOrId: vaultIdEncoded, secretName: secretName2 }); + // Header message + await writer.write({ + type: 'VaultNamesHeaderMessage', + vaultNames: [vaultIdEncoded], + }); + // Content messages + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded, + secretName: secretName1, + }); + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded, + secretName: invalidName, + }); + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded, + secretName: secretName2, + }); await writer.close(); let errorCount = 0; for await (const data of response.readable) { if (data.type === 'error') { - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'error'. - const error = data as ErrorMessage; // No other file name should raise this error - expect(error.reason).toStrictEqual(invalidName); + expect(data.reason).toStrictEqual(invalidName); errorCount++; continue; } @@ -2519,12 +2532,29 @@ describe('vaultsSecretsRemove', () => { // Delete secret const response = await rpcClient.methods.vaultsSecretsRemove(); const writer = response.writable.getWriter(); - await writer.write({ nameOrId: vaultIdEncoded, secretName: secretName1 }); - await writer.write({ nameOrId: vaultIdEncoded, secretName: secretName2 }); + // Header message + await writer.write({ + type: 'VaultNamesHeaderMessage', + vaultNames: [vaultIdEncoded], + }); + // Content messages + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded, + secretName: secretName1, + }); + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded, + secretName: secretName2, + }); await writer.close(); + let loopRun = false; for await (const data of response.readable) { + loopRun = true; expect(data.type).toStrictEqual('success'); } + expect(loopRun).toBeTruthy(); // Ensure single log message for deleting the secrets await vaultManager.withVaults([vaultId], async (vault) => { expect((await vault.log()).length).toEqual(logLength + 1); @@ -2555,14 +2585,35 @@ describe('vaultsSecretsRemove', () => { // Delete secret const response = await rpcClient.methods.vaultsSecretsRemove(); const writer = response.writable.getWriter(); - await writer.write({ nameOrId: vaultIdEncoded1, secretName: secretName1 }); - await writer.write({ nameOrId: vaultIdEncoded2, secretName: secretName2 }); - await writer.write({ nameOrId: vaultIdEncoded1, secretName: secretName3 }); + // Header message + await writer.write({ + type: 'VaultNamesHeaderMessage', + vaultNames: [vaultIdEncoded1, vaultIdEncoded2], + }); + // Content messages + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded1, + secretName: secretName1, + }); + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded2, + secretName: secretName2, + }); + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded1, + secretName: secretName3, + }); await writer.close(); + let loopRun = false; for await (const data of response.readable) { + loopRun = true; expect(data.type).toStrictEqual('success'); } // Ensure single log message for deleting the secrets + expect(loopRun).toBeTruthy(); await vaultManager.withVaults( [vaultId1, vaultId2], async (vault1, vault2) => { @@ -2595,10 +2646,17 @@ describe('vaultsSecretsRemove', () => { // Deleting directory with recursive set should not fail const response = await rpcClient.methods.vaultsSecretsRemove(); const writer = response.writable.getWriter(); + // Header message await writer.write({ + type: 'VaultNamesHeaderMessage', + vaultNames: [vaultIdEncoded], + recursive: true, + }); + // Content messages + await writer.write({ + type: 'SecretIdentifierMessage', nameOrId: vaultIdEncoded, secretName: dirName, - metadata: { options: { recursive: true } }, }); await writer.close(); for await (const data of response.readable) { @@ -2632,7 +2690,14 @@ describe('vaultsSecretsRemove', () => { // Deleting directory with recursive set should not fail const response = await rpcClient.methods.vaultsSecretsRemove(); const writer = response.writable.getWriter(); + // Header message await writer.write({ + type: 'VaultNamesHeaderMessage', + vaultNames: [vaultIdEncoded], + }); + // Content messages + await writer.write({ + type: 'SecretIdentifierMessage', nameOrId: vaultIdEncoded, secretName: dirName, }); From eb813db584b10fda18d2e3d88dde5aafbb96c0a6 Mon Sep 17 00:00:00 2001 From: Aryan Jassal Date: Fri, 29 Nov 2024 18:21:31 +1100 Subject: [PATCH 4/6] deps: updated rpc chore: removed local update to JSONRPCResponseResult --- package-lock.json | 8 ++++---- package.json | 2 +- src/client/types.ts | 13 +------------ 3 files changed, 6 insertions(+), 17 deletions(-) diff --git a/package-lock.json b/package-lock.json index 3649a90c1..bb93f336f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -21,7 +21,7 @@ "@matrixai/mdns": "^1.2.6", "@matrixai/quic": "^1.3.1", "@matrixai/resources": "^1.1.5", - "@matrixai/rpc": "^0.6.0", + "@matrixai/rpc": "^0.6.2", "@matrixai/timer": "^1.1.3", "@matrixai/workers": "^1.3.7", "@matrixai/ws": "^1.1.7", @@ -1696,9 +1696,9 @@ "integrity": "sha512-m/DEZEe3wHqWEPTyoBtzFF6U9vWYhEnQtGgwvqiAlTxTM0rk96UBpWjDZCTF/vYG11ZlmlQFtg5H+zGgbjaB3Q==" }, "node_modules/@matrixai/rpc": { - "version": "0.6.0", - "resolved": "https://registry.npmjs.org/@matrixai/rpc/-/rpc-0.6.0.tgz", - "integrity": "sha512-ENjJO2h7CmPLaHhObHs2nvpv98YZPxa79/jf+TqEPEfbhE1BkNCys9pXDE/CYDP9vxb4seS39WkR9cNivQU50A==", + "version": "0.6.3", + "resolved": "https://registry.npmjs.org/@matrixai/rpc/-/rpc-0.6.3.tgz", + "integrity": "sha512-jLR+SpAnv6NR2xRtXGBnyBGipLBv/Nnn/8b6OYx2loyNT4WHoxtqh51U6QwKCHU6qZii/K/2L5XRRKxYUgmmVg==", "dependencies": { "@matrixai/async-init": "^1.10.0", "@matrixai/contexts": "^1.2.0", diff --git a/package.json b/package.json index 998b13adc..d92e74101 100644 --- a/package.json +++ b/package.json @@ -80,7 +80,7 @@ "@matrixai/mdns": "^1.2.6", "@matrixai/quic": "^1.3.1", "@matrixai/resources": "^1.1.5", - "@matrixai/rpc": "^0.6.0", + "@matrixai/rpc": "^0.6.2", "@matrixai/timer": "^1.1.3", "@matrixai/workers": "^1.3.7", "@matrixai/ws": "^1.1.7", diff --git a/src/client/types.ts b/src/client/types.ts index 73d6ae271..c8843e334 100644 --- a/src/client/types.ts +++ b/src/client/types.ts @@ -1,8 +1,7 @@ import type { ClientManifest, JSONObject, - JSONRPCResponseMetadata, - // JSONRPCResponseResult, + JSONRPCResponseResult, RPCClient, } from '@matrixai/rpc'; import type { @@ -26,16 +25,6 @@ import type { } from '../nodes/types'; import type { AuditEventsGetTypeOverride } from './callers/auditEventsGet'; -// TEMP: For testing and debugging. This will go into js-rpc or something. -type JSONRPCResponseResult< - T extends JSONObject = JSONObject, - M extends JSONObject = JSONObject, -> = T & { - metadata?: JSONRPCResponseMetadata & - M & - (T extends { metadata: infer U } ? U : JSONObject); -}; - type ClientRPCRequestParams = JSONRPCResponseResult< T, From 6e322cec563b97287aa7377f0877b317481ec31a Mon Sep 17 00:00:00 2001 From: Aryan Jassal Date: Tue, 3 Dec 2024 09:47:34 +1100 Subject: [PATCH 5/6] chore: cleaned up tests and VaultsSecretsRemove fix: expand header message out of the function fix: input can be a AsyncIterableIterator --- src/client/handlers/VaultsSecretsCat.ts | 4 ++- src/client/handlers/VaultsSecretsEnv.ts | 4 +-- src/client/handlers/VaultsSecretsMkdir.ts | 2 +- src/client/handlers/VaultsSecretsRemove.ts | 34 ++++++++++++--------- src/git/utils.ts | 2 +- src/utils/utils.ts | 2 +- src/vaults/VaultInternal.ts | 2 +- tests/client/handlers/vaults.test.ts | 35 +++++++++++++++------- 8 files changed, 53 insertions(+), 32 deletions(-) diff --git a/src/client/handlers/VaultsSecretsCat.ts b/src/client/handlers/VaultsSecretsCat.ts index 2770b3b0b..e75be0c30 100644 --- a/src/client/handlers/VaultsSecretsCat.ts +++ b/src/client/handlers/VaultsSecretsCat.ts @@ -23,7 +23,9 @@ class VaultsSecretsCat extends DuplexHandler< ClientRPCResponseResult > { public handle = async function* ( - input: AsyncIterable>, + input: AsyncIterableIterator< + ClientRPCRequestParams + >, ): AsyncGenerator> { const { db, vaultManager }: { db: DB; vaultManager: VaultManager } = this.container; diff --git a/src/client/handlers/VaultsSecretsEnv.ts b/src/client/handlers/VaultsSecretsEnv.ts index acfc78119..58cb1b79d 100644 --- a/src/client/handlers/VaultsSecretsEnv.ts +++ b/src/client/handlers/VaultsSecretsEnv.ts @@ -10,7 +10,7 @@ import { DuplexHandler } from '@matrixai/rpc'; import * as vaultsUtils from '../../vaults/utils'; import * as vaultsErrors from '../../vaults/errors'; -class VaultsSecretsList extends DuplexHandler< +class VaultsSecretsEnv extends DuplexHandler< { db: DB; vaultManager: VaultManager; @@ -86,4 +86,4 @@ class VaultsSecretsList extends DuplexHandler< }; } -export default VaultsSecretsList; +export default VaultsSecretsEnv; diff --git a/src/client/handlers/VaultsSecretsMkdir.ts b/src/client/handlers/VaultsSecretsMkdir.ts index 9284c74e0..da8b2c00c 100644 --- a/src/client/handlers/VaultsSecretsMkdir.ts +++ b/src/client/handlers/VaultsSecretsMkdir.ts @@ -21,7 +21,7 @@ class VaultsSecretsMkdir extends DuplexHandler< ClientRPCResponseResult > { public handle = async function* ( - input: AsyncIterable>, + input: AsyncIterableIterator>, ): AsyncGenerator> { const { db, vaultManager }: { db: DB; vaultManager: VaultManager } = this.container; diff --git a/src/client/handlers/VaultsSecretsRemove.ts b/src/client/handlers/VaultsSecretsRemove.ts index bf57ba273..cbf4a7fb1 100644 --- a/src/client/handlers/VaultsSecretsRemove.ts +++ b/src/client/handlers/VaultsSecretsRemove.ts @@ -26,7 +26,7 @@ class VaultsSecretsRemove extends DuplexHandler< ClientRPCResponseResult > { public handle = async function* ( - input: AsyncIterable< + input: AsyncIterableIterator< ClientRPCRequestParams< SecretsRemoveHeaderMessage | SecretIdentifierMessageTagged > @@ -34,18 +34,19 @@ class VaultsSecretsRemove extends DuplexHandler< ): AsyncGenerator> { const { db, vaultManager }: { db: DB; vaultManager: VaultManager } = this.container; - const vaultAcquires: Array> = []; - // Extracts the header message from the iterator - const headerMessage = await (async () => { - const iterator = input[Symbol.asyncIterator](); - const header = (await iterator.next()).value; - if (header.type === 'VaultNamesHeaderMessage') { - if (header == null) throw new clientErrors.ErrorClientInvalidHeader(); - return header; - } - })(); + // Extract the header message from the iterator + const headerMessage: + | SecretsRemoveHeaderMessage + | SecretIdentifierMessageTagged = (await input.next()).value; + if ( + headerMessage == null || + headerMessage.type !== 'VaultNamesHeaderMessage' + ) { + throw new clientErrors.ErrorClientInvalidHeader(); + } // Create an array of write acquires - await db.withTransactionF(async (tran) => { + const vaultAcquires = await db.withTransactionF(async (tran) => { + const vaultAcquires: Array> = []; for (const vaultName of headerMessage.vaultNames) { const vaultIdFromName = await vaultManager.getVaultId(vaultName, tran); const vaultId = vaultIdFromName ?? vaultsUtils.decodeVaultId(vaultName); @@ -60,6 +61,7 @@ class VaultsSecretsRemove extends DuplexHandler< ); vaultAcquires.push(acquire); } + return vaultAcquires; }); // Acquire all locks in parallel and perform all operations at once yield* withG( @@ -72,7 +74,11 @@ class VaultsSecretsRemove extends DuplexHandler< } for await (const message of input) { // Ignoring any header messages - if (message.type !== 'SecretIdentifierMessage') continue; + if (message.type === 'VaultNamesHeaderMessage') { + throw new clientErrors.ErrorClientInvalidHeader( + 'The header message cannot be sent multiple times', + ); + } const efs = vaultMap.get(message.nameOrId); if (efs == null) { throw new vaultsErrors.ErrorVaultsVaultUndefined( @@ -98,7 +104,7 @@ class VaultsSecretsRemove extends DuplexHandler< e.code === 'ENOTEMPTY' || e.code === 'EINVAL' ) { - // INVAL can be triggered if removing the root of the + // EINVAL can be triggered if removing the root of the // vault is attempted. yield { type: 'error', diff --git a/src/git/utils.ts b/src/git/utils.ts index c80b956bc..cda1cbd94 100644 --- a/src/git/utils.ts +++ b/src/git/utils.ts @@ -218,7 +218,7 @@ async function listObjects({ } return; default: - utils.never(); + utils.never('Invalid type'); } } diff --git a/src/utils/utils.ts b/src/utils/utils.ts index cd4dbeda4..d14329739 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -48,7 +48,7 @@ function getDefaultNodePath(): string | undefined { return p; } -function never(message?: string): never { +function never(message: string): never { throw new utilsErrors.ErrorUtilsUndefinedBehaviour(message); } diff --git a/src/vaults/VaultInternal.ts b/src/vaults/VaultInternal.ts index b896d3ce1..2c700178d 100644 --- a/src/vaults/VaultInternal.ts +++ b/src/vaults/VaultInternal.ts @@ -571,7 +571,7 @@ class VaultInternal { } // The returned transaction can be undefined, too. We won't handle those // cases. - if (tran == null) utils.never(); + if (tran == null) utils.never('Acquired transactions cannot be null'); await tran.lock( [...this.vaultMetadataDbPath, VaultInternal.dirtyKey].join(''), ); diff --git a/tests/client/handlers/vaults.test.ts b/tests/client/handlers/vaults.test.ts index 57b256cb3..7b05bb6cc 100644 --- a/tests/client/handlers/vaults.test.ts +++ b/tests/client/handlers/vaults.test.ts @@ -1453,6 +1453,7 @@ describe('vaultsSecretsMkdir', () => { const vaultName = 'test-vault'; const vaultId = await vaultManager.createVault(vaultName); const dirPath = 'dir/dir1/dir2'; + // Attempt to make directories const response = await rpcClient.methods.vaultsSecretsMkdir(); const writer = response.writable.getWriter(); await writer.write({ @@ -1461,7 +1462,7 @@ describe('vaultsSecretsMkdir', () => { metadata: { options: { recursive: true } }, }); await writer.close(); - + // Check if the operation concluded as expected for await (const data of response.readable) { expect(data.type).toEqual('success'); } @@ -1476,13 +1477,15 @@ describe('vaultsSecretsMkdir', () => { const vaultId = await vaultManager.createVault(vaultName); const encodeVaultId = vaultsUtils.encodeVaultId(vaultId); const dirPath = 'dir/dir1/dir2'; + // Attempt to make directories const response = await rpcClient.methods.vaultsSecretsMkdir(); const writer = response.writable.getWriter(); await writer.write({ nameOrId: encodeVaultId, dirName: dirPath }); await writer.close(); + // Check if the operation concluded as expected for await (const data of response.readable) { expect(data.type).toEqual('error'); - if (data.type !== 'error') utils.never(); + if (data.type !== 'error') utils.never("Type is asserted to be 'error'"); expect(data.code).toEqual('ENOENT'); expect(data.reason).toEqual(dirPath); } @@ -1543,17 +1546,21 @@ describe('vaultsSecretsMkdir', () => { // Attempt to make directories const response = await rpcClient.methods.vaultsSecretsMkdir(); const writer = response.writable.getWriter(); - await writer.write({ nameOrId: vaultIdEncoded1, dirName: dirPath1 }); - await writer.write({ nameOrId: vaultIdEncoded2, dirName: dirPath2 }); await writer.write({ nameOrId: vaultIdEncoded1, dirName: dirPath3 }); + await writer.write({ nameOrId: vaultIdEncoded2, dirName: dirPath2 }); + await writer.write({ nameOrId: vaultIdEncoded1, dirName: dirPath1 }); await writer.close(); // Check if the operation concluded as expected + let successCount = 0; for await (const data of response.readable) { if (data.type === 'error') { expect(data.code).toEqual('ENOENT'); expect(data.reason).toEqual(dirPath3); + } else { + successCount++; } } + expect(successCount).toEqual(2); await vaultManager.withVaults( [vaultId1, vaultId2], async (vault1, vault2) => { @@ -1585,7 +1592,7 @@ describe('vaultsSecretsMkdir', () => { // Check if the operation concluded as expected for await (const data of response.readable) { expect(data.type).toEqual('error'); - if (data.type !== 'error') utils.never(); + if (data.type !== 'error') utils.never("Type is asserted to be 'error'"); expect(data.code).toEqual('EEXIST'); expect(data.reason).toEqual(dirPath); } @@ -1728,7 +1735,9 @@ describe('vaultsSecretsCat', () => { // Read response for await (const data of response.readable) { expect(data.type).toEqual('success'); - if (data.type !== 'success') utils.never(); + if (data.type !== 'success') { + utils.never("Type is asserted to be 'success'"); + } expect(data.secretContent).toEqual(secretContent); } }); @@ -1747,7 +1756,7 @@ describe('vaultsSecretsCat', () => { // Read response for await (const data of response.readable) { expect(data.type).toEqual('error'); - if (data.type !== 'error') utils.never(); + if (data.type !== 'error') utils.never("Type is asserted to be 'error'"); expect(data.code).toEqual('ENOENT'); expect(data.reason).toEqual(secretName); } @@ -1773,7 +1782,7 @@ describe('vaultsSecretsCat', () => { // Read response for await (const data of response.readable) { expect(data.type).toEqual('error'); - if (data.type !== 'error') utils.never(); + if (data.type !== 'error') utils.never("Type is asserted to be 'error'"); expect(data.code).toEqual('EISDIR'); expect(data.reason).toEqual(secretName); } @@ -1803,7 +1812,9 @@ describe('vaultsSecretsCat', () => { let totalContent = ''; for await (const data of response.readable) { expect(data.type).toEqual('success'); - if (data.type !== 'success') utils.never(); + if (data.type !== 'success') { + utils.never("Type is asserted to be 'success'"); + } totalContent += data.secretContent; } expect(totalContent).toEqual(`${secretContent1}${secretContent2}`); @@ -1845,7 +1856,9 @@ describe('vaultsSecretsCat', () => { let totalContent = ''; for await (const data of response.readable) { expect(data.type).toEqual('success'); - if (data.type !== 'success') utils.never(); + if (data.type !== 'success') { + utils.never("Type is asserted to be 'success'"); + } totalContent += data.secretContent; } expect(totalContent).toEqual( @@ -2397,7 +2410,7 @@ describe('vaultsSecretsRemove', () => { for await (const data of response.readable) { loopRun = true; expect(data.type).toStrictEqual('error'); - if (data.type !== 'error') utils.never(); + if (data.type !== 'error') utils.never("Type is asserted to be 'error'"); expect(data.code).toStrictEqual('EINVAL'); } // Check From 74911444f542930e6acf91ee4d52d401ddad95f5 Mon Sep 17 00:00:00 2001 From: Aryan Jassal Date: Tue, 10 Dec 2024 12:06:17 +1100 Subject: [PATCH 6/6] chore: added tests for client protocol --- src/client/errors.ts | 6 ++ src/client/handlers/VaultsSecretsRemove.ts | 18 +++-- src/git/utils.ts | 4 +- src/vaults/VaultInternal.ts | 3 - tests/audit/utils.test.ts | 1 + tests/client/handlers/nodes.test.ts | 1 - tests/client/handlers/vaults.test.ts | 79 +++++++++++++++++++++- tests/git/utils.ts | 6 +- 8 files changed, 105 insertions(+), 13 deletions(-) diff --git a/src/client/errors.ts b/src/client/errors.ts index 7d5ca751a..618f4cd1d 100644 --- a/src/client/errors.ts +++ b/src/client/errors.ts @@ -23,6 +23,11 @@ class ErrorClientInvalidHeader extends ErrorClient { exitCode = sysexits.USAGE; } +class ErrorClientProtocolError extends ErrorClient { + static description = 'Data does not match the protocol requirements'; + exitCode = sysexits.USAGE; +} + class ErrorClientService extends ErrorClient {} class ErrorClientServiceRunning extends ErrorClientService { @@ -51,6 +56,7 @@ export { ErrorClientAuthFormat, ErrorClientAuthDenied, ErrorClientInvalidHeader, + ErrorClientProtocolError, ErrorClientService, ErrorClientServiceRunning, ErrorClientServiceNotRunning, diff --git a/src/client/handlers/VaultsSecretsRemove.ts b/src/client/handlers/VaultsSecretsRemove.ts index cbf4a7fb1..eb8796ef5 100644 --- a/src/client/handlers/VaultsSecretsRemove.ts +++ b/src/client/handlers/VaultsSecretsRemove.ts @@ -35,11 +35,13 @@ class VaultsSecretsRemove extends DuplexHandler< const { db, vaultManager }: { db: DB; vaultManager: VaultManager } = this.container; // Extract the header message from the iterator + const headerMessagePair = await input.next(); const headerMessage: | SecretsRemoveHeaderMessage - | SecretIdentifierMessageTagged = (await input.next()).value; + | SecretIdentifierMessageTagged = headerMessagePair.value; + // Testing if the header is of the expected format if ( - headerMessage == null || + headerMessagePair.done || headerMessage.type !== 'VaultNamesHeaderMessage' ) { throw new clientErrors.ErrorClientInvalidHeader(); @@ -72,10 +74,12 @@ class VaultsSecretsRemove extends DuplexHandler< for (let i = 0; i < efses.length; i++) { vaultMap.set(headerMessage!.vaultNames[i], efses[i]); } + let loopRan = false; for await (const message of input) { - // Ignoring any header messages + loopRan = true; + // Header messages should not be seen anymore if (message.type === 'VaultNamesHeaderMessage') { - throw new clientErrors.ErrorClientInvalidHeader( + throw new clientErrors.ErrorClientProtocolError( 'The header message cannot be sent multiple times', ); } @@ -116,6 +120,12 @@ class VaultsSecretsRemove extends DuplexHandler< } } } + // Content messages must follow header messages + if (!loopRan) { + throw new clientErrors.ErrorClientProtocolError( + 'No content messages followed header message', + ); + } }, ); }; diff --git a/src/git/utils.ts b/src/git/utils.ts index cda1cbd94..dd5f914d6 100644 --- a/src/git/utils.ts +++ b/src/git/utils.ts @@ -218,7 +218,9 @@ async function listObjects({ } return; default: - utils.never('Invalid type'); + utils.never( + `type must be one of "commit", "tree", "blob", or "tag", got "${type}"`, + ); } } diff --git a/src/vaults/VaultInternal.ts b/src/vaults/VaultInternal.ts index 2c700178d..af778590d 100644 --- a/src/vaults/VaultInternal.ts +++ b/src/vaults/VaultInternal.ts @@ -596,9 +596,6 @@ class VaultInternal { await this.createCommit(); } catch (e_) { e = e_; - } - // This would happen if an error was caught inside the catch block - if (e != null) { // Error implies dirty state await this.cleanWorkingDirectory(); } diff --git a/tests/audit/utils.test.ts b/tests/audit/utils.test.ts index ce9f1c9b7..6555b334d 100644 --- a/tests/audit/utils.test.ts +++ b/tests/audit/utils.test.ts @@ -47,6 +47,7 @@ describe('Audit Utils', () => { ['f'], ['f'], ]; + // @ts-ignore: treating TopicSubPath as string for testing const filtered = auditUtils.filterSubPaths(data).map((v) => v.join('.')); expect(filtered).toHaveLength(3); expect(filtered).toInclude('a.b'); diff --git a/tests/client/handlers/nodes.test.ts b/tests/client/handlers/nodes.test.ts index ff1e85a11..7a084039a 100644 --- a/tests/client/handlers/nodes.test.ts +++ b/tests/client/handlers/nodes.test.ts @@ -825,7 +825,6 @@ describe('nodesGetAll', () => { manifest: { nodesGetAll: new NodesGetAll({ nodeGraph, - keyRing, }), }, host: localhost, diff --git a/tests/client/handlers/vaults.test.ts b/tests/client/handlers/vaults.test.ts index 7b05bb6cc..326bea568 100644 --- a/tests/client/handlers/vaults.test.ts +++ b/tests/client/handlers/vaults.test.ts @@ -70,6 +70,7 @@ import * as keysUtils from '@/keys/utils'; import * as nodesUtils from '@/nodes/utils'; import * as vaultsUtils from '@/vaults/utils'; import * as vaultsErrors from '@/vaults/errors'; +import * as clientErrors from '@/client/errors'; import * as networkUtils from '@/network/utils'; import * as utils from '@/utils'; import * as testsUtils from '../../utils'; @@ -2356,6 +2357,80 @@ describe('vaultsSecretsRemove', () => { recursive: true, }); }); + test('fails when header is not sent', async () => { + // Write paths + const response = await rpcClient.methods.vaultsSecretsRemove(); + const writer = response.writable.getWriter(); + // Not sending the header message + // Content messages + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: 'invalid', + secretName: 'invalid', + }); + await writer.close(); + // Read response + const consumeP = async () => { + for await (const _ of response.readable) { + // Consume values + } + }; + await testsUtils.expectRemoteError( + consumeP(), + clientErrors.ErrorClientInvalidHeader, + ); + }); + test('fails when only the header is sent', async () => { + const vaultId = await vaultManager.createVault('test-vault'); + const vaultIdEncoded = vaultsUtils.encodeVaultId(vaultId); + // Write paths + const response = await rpcClient.methods.vaultsSecretsRemove(); + const writer = response.writable.getWriter(); + // Header message + await writer.write({ + type: 'VaultNamesHeaderMessage', + vaultNames: [vaultIdEncoded], + }); + // Not sending the content messages + await writer.close(); + // Read response + const consumeP = async () => { + for await (const _ of response.readable) { + // Consume values + } + }; + await testsUtils.expectRemoteError( + consumeP(), + clientErrors.ErrorClientProtocolError, + ); + }); + test('fails when the header is sent multiple times', async () => { + const vaultId = await vaultManager.createVault('test-vault'); + const vaultIdEncoded = vaultsUtils.encodeVaultId(vaultId); + // Write paths + const response = await rpcClient.methods.vaultsSecretsRemove(); + const writer = response.writable.getWriter(); + // Header message + await writer.write({ + type: 'VaultNamesHeaderMessage', + vaultNames: [vaultIdEncoded], + }); + await writer.write({ + type: 'VaultNamesHeaderMessage', + vaultNames: [vaultIdEncoded], + }); + await writer.close(); + // Read response + const consumeP = async () => { + for await (const _ of response.readable) { + // Consume values + } + }; + await testsUtils.expectRemoteError( + consumeP(), + clientErrors.ErrorClientProtocolError, + ); + }); test('fails with invalid vault name', async () => { // Write paths const response = await rpcClient.methods.vaultsSecretsRemove(); @@ -2374,7 +2449,9 @@ describe('vaultsSecretsRemove', () => { await writer.close(); // Read response const consumeP = async () => { - for await (const _ of response.readable); + for await (const _ of response.readable) { + // Consume values + } }; await testsUtils.expectRemoteError( consumeP(), diff --git a/tests/git/utils.ts b/tests/git/utils.ts index 8205ebb90..1311208ae 100644 --- a/tests/git/utils.ts +++ b/tests/git/utils.ts @@ -90,7 +90,8 @@ type NegotiationTestData = * @param rest - Random buffer data to be appended to the end to simulate more lines in the stream. */ function generateGitNegotiationLine(data: NegotiationTestData, rest: Buffer) { - switch (data.type) { + const type = data.type; + switch (type) { case 'want': { // Generate a `want` line that includes `want`, the `objectId` and capabilities const line = Buffer.concat([ @@ -123,9 +124,8 @@ function generateGitNegotiationLine(data: NegotiationTestData, rest: Buffer) { // Generate an empty buffer to simulate the stream running out of data to process return Buffer.alloc(0); default: - // @ts-ignore: if we're here then data isn't the type we expect utils.never( - `data.type must be "want", "have", "SEPARATOR", "done", "none", got "${data.type}"`, + `data.type must be "want", "have", "SEPARATOR", "done", "none", got "${type}"`, ); } }