Adding secrets rm command

[aryanjassal]

Description

Implement secrets rm command following the behaviour of UNIX. For example, passing the -r flag would recursively delete a directory and it's contents, and we can supply multiple secret paths to the command.

Issues Fixed

• Related to Implement secrets rm command #247 (ENG-360)
• Related to Implementing secrets rm RPC handler Polykey#803
• Closes ENG-360

Tasks

• 1. Add new secrets rm command (functionally similar to secrets del)
• 2. Add support for specifying and deleting multiple files
• 3. Supply --recursive to delete directories and their contents
• 4. Add tests

Final checklist

• Domain specific tests
• Full tests
• Updated inline-comment documentation
• Lint fixed
• Squash and rebased
• Sanity check the final build

[linear]

ENG-360 Implement `secrets rm` command

[tegefaulkes]

I've found some small problems.

[CMCDragonkai]

Yes in that case -f is useless don't put that in. Add in -r though.

[CMCDragonkai]

It's about consistent semantics not just convenience.

[aryanjassal]

Polykey-CLI/tests/secrets/stat.test.ts

Lines 51 to 64 in 40fb687

	command = [
	'secrets',
	'stat',
	'-np',
	dataDir,
	`${vaultName}:MySecret`,
	'--format',
	'json',
	];
	const result = await testUtils.pkStdio([...command], {
	env: { PK_PASSWORD: password },
	cwd: dataDir,
	});

I've noticed something interesting. In almost every test, we define command as a list of commands. But, for some reason, we unwrap the list, then wrap the contents into an array again, like [...command]. It can all be simplified to simply passing in the array directly, like this:

const command = ['secrets', 'rm', '-np', dataDir, `${vaultName}:secret`];

const result = await testUtils.pkStdio(command, {
  env: { PK_PASSWORD: password },
  cwd: dataDir,
});

This would have the result of making the tests slightly faster and more readable. I have also noticed that in some tests, command is defined globally, and in others, it is defined as a const for each test (see file linked above for this behaviour). I think we should define it in each test to ensure the command remains scoped to that test.

[aryanjassal]

All the tasks have been finished for this PR and approval has also been granted. Merging.

[CMCDragonkai]

Polykey-CLI/tests/secrets/stat.test.ts

Lines 51 to 64 in 40fb687

	command = [
	'secrets',
	'stat',
	'-np',
	dataDir,
	`${vaultName}:MySecret`,
	'--format',
	'json',
	];
	const result = await testUtils.pkStdio([...command], {
	env: { PK_PASSWORD: password },
	cwd: dataDir,
	});

I've noticed something interesting. In almost every test, we define command as a list of commands. But, for some reason, we unwrap the list, then wrap the contents into an array again, like [...command]. It can all be simplified to simply passing in the array directly, like this:

const command = ['secrets', 'rm', '-np', dataDir, `${vaultName}:secret`];

const result = await testUtils.pkStdio(command, {
  env: { PK_PASSWORD: password },
  cwd: dataDir,
});

This would have the result of making the tests slightly faster and more readable. I have also noticed that in some tests, command is defined globally, and in others, it is defined as a const for each test (see file linked above for this behaviour). I think we should define it in each test to ensure the command remains scoped to that test.

Spreading is usually intended for shallow copies. Not sure what that's for.

[aryanjassal]

[CMCDragonkai]

Interesting it's 1 commit message.

Do note to use ls not list in order to be consistent in our messaging.