Skip to content

Commit 8d9e54b

Browse files
aryanjassalaryanjassal
committed
chore: cleaned up logic
chore: updated help text
1 parent ef7d18e commit 8d9e54b

File tree

3 files changed

+69
-142
lines changed

3 files changed

+69
-142
lines changed

‎.gitignore‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,3 +130,6 @@ dist
130130
# editor
131131
.vscode/
132132
.idea/
133+
134+
# prettier for editor formatting
135+
.prettierrc

‎src/secrets/CommandRename.ts‎

Lines changed: 20 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -16,29 +16,32 @@ class CommandRename extends CommandPolykey {
1616
'Path to where the secret to be renamed, specified as <vaultName>:<directoryPath>',
1717
binParsers.parseSecretPath,
1818
);
19-
this.argument('<newSecretName>', 'New name of the secret');
19+
this.argument(
20+
'<newSecretName>',
21+
'Fully-qualified new name for the secret, specified as <vaultName>:<secretPath>',
22+
binParsers.parseSecretPath,
23+
);
2024
this.addOption(binOptions.nodeId);
2125
this.addOption(binOptions.clientHost);
2226
this.addOption(binOptions.clientPort);
23-
this.action(async (secretPath, newSecretNameArg, options) => {
27+
this.action(async (secretPath, newSecretPath, options) => {
28+
// Rename operation cannot work across vaults, only within a vault
29+
if (secretPath[0] !== newSecretPath[0]) {
30+
throw new errors.ErrorPolykeyCLIRenameSecret(
31+
'Cannot rename file into another vault',
32+
);
33+
}
2434
// Ensure that a valid secret path is provided
25-
if (
26-
secretPath[1] == null ||
27-
secretPath[1].trim() === '' ||
28-
secretPath[1].trim() === '/'
29-
) {
35+
if (secretPath[1] == null) {
3036
throw new errors.ErrorPolykeyCLIRenameSecret(
31-
'EPERM: Cannot rename vault root',
37+
'Cannot rename vault root',
38+
);
39+
}
40+
if (newSecretPath[1] == null) {
41+
throw new errors.ErrorPolykeyCLIRenameSecret(
42+
'Cannot rename to vault root',
3243
);
3344
}
34-
35-
// Process the newSecretNameArg using the abstracted helper method.
36-
// secretPath[0] is the original vault name.
37-
const finalNewSecretName = CommandRename._processNewSecretNameArgument(
38-
newSecretNameArg,
39-
secretPath[0],
40-
);
41-
4245
const { default: PolykeyClient } = await import(
4346
'polykey/PolykeyClient.js'
4447
);
@@ -75,7 +78,7 @@ class CommandRename extends CommandPolykey {
7578
metadata: auth,
7679
nameOrId: secretPath[0],
7780
secretName: secretPath[1],
78-
newSecretName: finalNewSecretName,
81+
newSecretName: newSecretPath[1],
7982
}),
8083
meta,
8184
);
@@ -84,67 +87,6 @@ class CommandRename extends CommandPolykey {
8487
}
8588
});
8689
}
87-
88-
/**
89-
* Processes the newSecretName argument to ensure it's a valid base name.
90-
*/
91-
private static _processNewSecretNameArgument(
92-
newSecretNameArg: string,
93-
originalVaultName: string,
94-
): string {
95-
let finalNewBaseName = newSecretNameArg;
96-
97-
if (newSecretNameArg.includes(':')) {
98-
let parsedNewPath: Array<any>;
99-
try {
100-
parsedNewPath = binParsers.parseSecretPath(newSecretNameArg);
101-
} catch (e: any) {
102-
throw new errors.ErrorPolykeyCLIRenameSecret(
103-
`EINVALID: The new secret name '${newSecretNameArg}' appears to be a path but could not be parsed: ${e.message}`,
104-
);
105-
}
106-
const newVaultNameInArg: string = parsedNewPath[0];
107-
const newSecretPathPart: string = parsedNewPath[1];
108-
if (newVaultNameInArg !== originalVaultName) {
109-
throw new errors.ErrorPolykeyCLIRenameSecret(
110-
`ECROSSVAULT: Renaming to a different vault ('${newVaultNameInArg}') is not supported by this command. The target vault must be the same as the source vault ('${originalVaultName}').`,
111-
);
112-
}
113-
if (
114-
newSecretPathPart == null ||
115-
newSecretPathPart.trim() === '' ||
116-
newSecretPathPart.trim() === '/'
117-
) {
118-
throw new errors.ErrorPolykeyCLIRenameSecret(
119-
`EINVALID: The path component of the new secret name '${newSecretNameArg}' is empty or invalid.`,
120-
);
121-
}
122-
const parts = newSecretPathPart.split('/').filter((p) => p.length > 0);
123-
if (parts.length === 0) {
124-
throw new errors.ErrorPolykeyCLIRenameSecret(
125-
`EINVALID: Could not extract a valid base name from the path component '${newSecretPathPart}' in '${newSecretNameArg}'.`,
126-
);
127-
}
128-
finalNewBaseName = parts[parts.length - 1];
129-
} else {
130-
if (newSecretNameArg.includes('/')) {
131-
throw new errors.ErrorPolykeyCLIRenameSecret(
132-
`EINVALIDNAME: The new secret name '${newSecretNameArg}' must be a base name and cannot contain '/'. If you intended to specify a path, include the vault name (e.g., '${originalVaultName}:/path/NewName').`,
133-
);
134-
}
135-
}
136-
if (!finalNewBaseName || finalNewBaseName.trim() === '') {
137-
throw new errors.ErrorPolykeyCLIRenameSecret(
138-
`EEMPTYNAME: The new secret name derived from '${newSecretNameArg}' is empty.`,
139-
);
140-
}
141-
if (finalNewBaseName.includes('/') || finalNewBaseName.includes(':')) {
142-
throw new errors.ErrorPolykeyCLIRenameSecret(
143-
`EINVALIDCHAR: The final new secret name '${finalNewBaseName}' (derived from '${newSecretNameArg}') is invalid. It cannot contain '/' or ':' characters.`,
144-
);
145-
}
146-
return finalNewBaseName;
147-
}
14890
}
14991

15092
export default CommandRename;

‎tests/secrets/rename.test.ts‎

Lines changed: 46 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ describe('commandRenameSecret', () => {
1212
const logger = new Logger('CLI Test', LogLevel.WARN, [new StreamHandler()]);
1313
let dataDir: string;
1414
let polykeyAgent: PolykeyAgent;
15-
let command: Array<string>;
1615

1716
beforeEach(async () => {
1817
dataDir = await fs.promises.mkdtemp(
@@ -42,106 +41,89 @@ describe('commandRenameSecret', () => {
4241
});
4342
});
4443

45-
test('should rename secrets using a simple new name', async () => {
46-
const vaultName = 'vaultSimpleRename' as VaultName;
44+
test('should rename secrets', async () => {
45+
const vaultName = 'vault' as VaultName;
4746
const vaultId = await polykeyAgent.vaultManager.createVault(vaultName);
48-
const oldSecretName = 'secretOriginal';
49-
const newSecretBaseName = 'secretNewBase'; // Changed variable name for clarity
47+
const oldSecretName = 'secretOld';
48+
const newSecretName = 'secretNew';
5049
const secretContent = 'this is the secret for simple rename';
5150
await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
5251
await vaultOps.addSecret(vault, oldSecretName, secretContent);
5352
});
54-
command = [
53+
54+
// Should fail if only new name is provided
55+
const command1 = [
5556
'secrets',
5657
'rename',
5758
'-np',
5859
dataDir,
5960
`${vaultName}:${oldSecretName}`,
60-
newSecretBaseName, // Use the simple base name
61+
newSecretName,
6162
];
62-
const result = await testUtils.pkStdio(command, {
63+
const result1 = await testUtils.pkStdio(command1, {
6364
env: { PK_PASSWORD: password },
6465
cwd: dataDir,
6566
});
66-
expect(result.exitCode).toBe(0);
67-
expect(result.stderr).toBe(''); // Expect no errors
67+
expect(result1.exitCode).toBe(1);
6868
await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
6969
const list = await vaultOps.listSecrets(vault);
70-
expect(list.sort()).toStrictEqual([newSecretBaseName]);
71-
expect(list).not.toContain(oldSecretName);
70+
expect(list.sort()).toStrictEqual([oldSecretName]);
7271
});
73-
});
7472

75-
// New test case for the fix
76-
test('should rename secret when new name is a fully qualified path', async () => {
77-
const vaultName = 'vaultFQRename' as VaultName; // Use a distinct vault name for the test
78-
const vaultId = await polykeyAgent.vaultManager.createVault(vaultName);
79-
const oldSecretName = 'oldSecretName';
80-
const newSecretBaseName = 'newSecretNameByPath'; // The actual target base name
81-
const secretContent = 'content for fully qualified path rename test';
82-
83-
// Add initial secret
84-
await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
85-
await vaultOps.addSecret(vault, oldSecretName, secretContent);
86-
});
87-
88-
// Construct the fully qualified path for the new secret name
89-
// This format matches the problematic case: VaultName:/NewName
90-
const newSecretFullyQualified = `${vaultName}:/${newSecretBaseName}`;
91-
92-
command = [
73+
// Should pass if fully-qualified path is provided
74+
const command2 = [
9375
'secrets',
9476
'rename',
9577
'-np',
9678
dataDir,
97-
`${vaultName}:${oldSecretName}`, // Source secret path
98-
newSecretFullyQualified, // New secret name as fully qualified path
79+
`${vaultName}:${oldSecretName}`,
80+
`${vaultName}:${newSecretName}`,
9981
];
100-
101-
const result = await testUtils.pkStdio(command, {
82+
const result2 = await testUtils.pkStdio(command2, {
10283
env: { PK_PASSWORD: password },
10384
cwd: dataDir,
10485
});
105-
106-
expect(result.exitCode).toBe(0);
107-
expect(result.stderr).toBe(''); // Expect no errors
108-
109-
// Verify the rename
86+
expect(result2.exitCode).toBe(0);
11087
await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
111-
const secretsList = await vaultOps.listSecrets(vault);
112-
expect(secretsList).toContain(newSecretBaseName); // Check for the base name
113-
expect(secretsList).not.toContain(oldSecretName);
114-
expect(secretsList.length).toBe(1); // Assuming only this secret is in the test vault
88+
const list = await vaultOps.listSecrets(vault);
89+
expect(list.sort()).toStrictEqual([newSecretName]);
11590
});
11691
});
11792

118-
test('should not rename vault root', async () => {
119-
const vaultName = 'vaultRootTest' as VaultName; // Use a distinct vault name
120-
await polykeyAgent.vaultManager.createVault(vaultName);
121-
// Attempting to rename the vault itself, or an empty path within it
122-
command = [
93+
test('should fail renaming across vaults', async () => {
94+
const vaultName1 = 'vault1' as VaultName;
95+
const vaultName2 = 'vault2' as VaultName;
96+
const vaultId1 = await polykeyAgent.vaultManager.createVault(vaultName1);
97+
const vaultId2 = await polykeyAgent.vaultManager.createVault(vaultName2);
98+
const oldSecretName = 'secretOld';
99+
const newSecretName = 'secretNew';
100+
const secretContent = 'this is the secret for simple rename';
101+
await polykeyAgent.vaultManager.withVaults([vaultId1], async (vault) => {
102+
await vaultOps.addSecret(vault, oldSecretName, secretContent);
103+
});
104+
const command = [
123105
'secrets',
124106
'rename',
125107
'-np',
126108
dataDir,
127-
`${vaultName}:/`,
128-
'newNameForRoot',
109+
`${vaultName1}:${oldSecretName}`,
110+
`${vaultName2}:${newSecretName}`,
129111
];
130-
let result = await testUtils.pkStdio(command, {
131-
env: { PK_PASSWORD: password },
132-
cwd: dataDir,
133-
});
134-
expect(result.exitCode).not.toBe(0);
135-
expect(result.stderr).toInclude('EPERM');
136-
137-
// Original test for trying to rename the vault name directly (which parseSecretPath handles as [vaultName, undefined, undefined])
138-
// The `CommandRename` checks secretPath[1] == null, which covers `vaultName` (no colon) for the first arg.
139-
command = ['secrets', 'rename', '-np', dataDir, vaultName, 'rename'];
140-
result = await testUtils.pkStdio(command, {
112+
const result = await testUtils.pkStdio(command, {
141113
env: { PK_PASSWORD: password },
142114
cwd: dataDir,
143115
});
144-
expect(result.exitCode).not.toBe(0);
145-
expect(result.stderr).toInclude('EPERM'); // This is because secretPath[1] will be undefined
116+
expect(result.exitCode).toBe(1);
117+
await polykeyAgent.vaultManager.withVaults(
118+
[vaultId1, vaultId2],
119+
async (vault1, vault2) => {
120+
// The secret wasn't changed in the original place
121+
const list1 = await vaultOps.listSecrets(vault1);
122+
expect(list1.sort()).toStrictEqual([oldSecretName]);
123+
// The target vault is also unchanged
124+
const list2 = await vaultOps.listSecrets(vault2);
125+
expect(list2.sort()).toStrictEqual([]);
126+
},
127+
);
146128
});
147129
});

0 commit comments

Comments
 (0)