Skip to content

Commit 9f7a7e1

Browse files
aryanjassalaryanjassal
authored
Merge pull request #267 from MatrixAI/feature-unix-edit
Now, `secrets edit` creates a secret if it doesn't exist in the vault
2 parents a031b1b + c2682d6 commit 9f7a7e1

4 files changed

Lines changed: 259 additions & 37 deletions

File tree

‎src/secrets/CommandEdit.ts‎

Lines changed: 78 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import type PolykeyClient from 'polykey/dist/PolykeyClient';
22
import fs from 'fs';
33
import path from 'path';
4-
import * as errors from '../errors';
54
import CommandPolykey from '../CommandPolykey';
5+
import * as errors from '../errors';
66
import * as binUtils from '../utils';
77
import * as binOptions from '../utils/options';
88
import * as binParsers from '../utils/parsers';
@@ -24,6 +24,7 @@ class CommandEdit extends CommandPolykey {
2424
this.action(async (secretPath, options) => {
2525
const os = await import('os');
2626
const { execSync } = await import('child_process');
27+
const vaultsErrors = await import('polykey/dist/vaults/errors');
2728
const { default: PolykeyClient } = await import(
2829
'polykey/dist/PolykeyClient'
2930
);
@@ -44,6 +45,10 @@ class CommandEdit extends CommandPolykey {
4445
this.exitHandlers.handlers.push(async () => {
4546
if (pkClient != null) await pkClient.stop();
4647
});
48+
49+
const tmpDir = await fs.promises.mkdtemp(
50+
path.join(os.tmpdir(), 'polykey-'),
51+
);
4752
try {
4853
pkClient = await PolykeyClient.createPolykeyClient({
4954
nodeId: clientOptions.nodeId,
@@ -54,45 +59,86 @@ class CommandEdit extends CommandPolykey {
5459
},
5560
logger: this.logger.getChild(PolykeyClient.name),
5661
});
57-
const response = await binUtils.retryAuthentication(
58-
(auth) =>
59-
pkClient.rpcClient.methods.vaultsSecretsGet({
60-
metadata: auth,
61-
nameOrId: secretPath[0],
62-
secretName: secretPath[1],
63-
}),
62+
const tmpFile = path.join(tmpDir, path.basename(secretPath[1]));
63+
const secretExists = await binUtils.retryAuthentication(
64+
async (auth) => {
65+
let exists: boolean = true;
66+
try {
67+
const response =
68+
await pkClient.rpcClient.methods.vaultsSecretsGet({
69+
metadata: auth,
70+
nameOrId: secretPath[0],
71+
secretName: secretPath[1],
72+
});
73+
await this.fs.promises.writeFile(tmpFile, response.secretContent);
74+
} catch (e) {
75+
const [cause, _] = binUtils.remoteErrorCause(e);
76+
if (cause instanceof vaultsErrors.ErrorSecretsSecretUndefined) {
77+
exists = false;
78+
} else {
79+
throw e;
80+
}
81+
}
82+
return exists;
83+
},
6484
meta,
6585
);
66-
const secretContent = response.secretContent;
67-
const tmpDir = await fs.promises.mkdtemp(
68-
path.join(os.tmpdir(), 'polykey-'),
69-
);
70-
const tmpFile = path.join(tmpDir, 'pksecret');
71-
await this.fs.promises.writeFile(tmpFile, secretContent);
72-
execSync(`$EDITOR \"${tmpFile}\"`, { stdio: 'inherit' });
73-
let content: Buffer;
86+
// If the editor exited with a code other than zero, then execSync
87+
// will throw an error. So, in the case of saving the file but the
88+
// editor crashing, the program won't save the updated secret.
89+
execSync(`${process.env.EDITOR} \"${tmpFile}\"`, { stdio: 'inherit' });
90+
let content: string;
7491
try {
75-
content = await this.fs.promises.readFile(tmpFile);
92+
content = (await this.fs.promises.readFile(tmpFile)).toString(
93+
'binary',
94+
);
7695
} catch (e) {
77-
throw new errors.ErrorPolykeyCLIFileRead(e.message, {
78-
data: {
79-
errno: e.errno,
80-
syscall: e.syscall,
81-
code: e.code,
82-
path: e.path,
83-
},
84-
cause: e,
85-
});
96+
if (e.code === 'ENOENT') {
97+
// If the secret exists but the file doesn't, then something went
98+
// wrong, and the file cannot be read anymore. This is bad.
99+
if (secretExists) {
100+
throw new errors.ErrorPolykeyCLIFileRead(e.message, {
101+
data: {
102+
errno: e.errno,
103+
syscall: e.syscall,
104+
code: e.code,
105+
path: e.path,
106+
},
107+
cause: e,
108+
});
109+
// If the secret didn't exist before and we can't read the file,
110+
// then the secret was never actually created or saved. The user
111+
// doesn't want to make the secret anymore, so abort mision!
112+
} else {
113+
return;
114+
}
115+
}
116+
throw e;
86117
}
87-
await pkClient.rpcClient.methods.vaultsSecretsEdit({
88-
nameOrId: secretPath[0],
89-
secretName: secretPath[1],
90-
secretContent: content.toString('binary'),
91-
});
92-
await this.fs.promises.rm(tmpDir, { recursive: true, force: true });
118+
await binUtils.retryAuthentication(async (auth) => {
119+
// This point will never be reached if the temp file doesn't exist.
120+
// As such, if the secret didn't exist before, then we want to make it.
121+
// Otherwise, if the secret existed before, then we want to edit it.
122+
if (secretExists) {
123+
await pkClient.rpcClient.methods.vaultsSecretsEdit({
124+
metadata: auth,
125+
nameOrId: secretPath[0],
126+
secretName: secretPath[1],
127+
secretContent: content,
128+
});
129+
} else {
130+
await pkClient.rpcClient.methods.vaultsSecretsNew({
131+
metadata: auth,
132+
nameOrId: secretPath[0],
133+
secretName: secretPath[1],
134+
secretContent: content,
135+
});
136+
}
137+
}, meta);
93138
// Windows
94139
// TODO: complete windows impl
95140
} finally {
141+
await this.fs.promises.rm(tmpDir, { recursive: true, force: true });
96142
if (pkClient! != null) await pkClient.stop();
97143
}
98144
});

‎tests/secrets/edit.test.ts‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
import type { VaultName } from 'polykey/dist/vaults/types';
2+
import path from 'path';
3+
import fs from 'fs';
4+
import Logger, { LogLevel, StreamHandler } from '@matrixai/logger';
5+
import PolykeyAgent from 'polykey/dist/PolykeyAgent';
6+
import { vaultOps } from 'polykey/dist/vaults';
7+
import * as keysUtils from 'polykey/dist/keys/utils';
8+
import * as testUtils from '../utils';
9+
10+
describe('commandEditSecret', () => {
11+
const password = 'password';
12+
const logger = new Logger('CLI Test', LogLevel.WARN, [new StreamHandler()]);
13+
const editedContent = 'edited secret contents';
14+
let dataDir: string;
15+
let editorEdit: string;
16+
let editorExit: string;
17+
let editorFail: string;
18+
let editorView: string;
19+
let polykeyAgent: PolykeyAgent;
20+
let command: Array<string>;
21+
22+
beforeEach(async () => {
23+
dataDir = await fs.promises.mkdtemp(
24+
path.join(globalThis.tmpDir, 'polykey-test-'),
25+
);
26+
editorEdit = path.join(dataDir, 'editorEdit.sh');
27+
editorExit = path.join(dataDir, 'editorExit.sh');
28+
editorFail = path.join(dataDir, 'editorFail.sh');
29+
editorView = path.join(dataDir, 'editorView.sh');
30+
await fs.promises.writeFile(editorExit, `#!/usr/bin/env bash\nexit`);
31+
await fs.promises.chmod(editorExit, '755');
32+
await fs.promises.writeFile(
33+
editorView,
34+
`#!/usr/bin/env bash\ncp $1 ${dataDir}/secret; echo "${editedContent}" > $1`,
35+
);
36+
await fs.promises.chmod(editorView, '755');
37+
await fs.promises.writeFile(
38+
editorEdit,
39+
`#!/usr/bin/env bash\necho "${editedContent}" > $1`,
40+
);
41+
await fs.promises.chmod(editorEdit, '755');
42+
await fs.promises.writeFile(
43+
editorFail,
44+
`#!/usr/bin/env bash\necho "${editedContent}" > $1; exit 1`,
45+
);
46+
await fs.promises.chmod(editorFail, '755');
47+
polykeyAgent = await PolykeyAgent.createPolykeyAgent({
48+
password,
49+
options: {
50+
nodePath: dataDir,
51+
agentServiceHost: '127.0.0.1',
52+
clientServiceHost: '127.0.0.1',
53+
keys: {
54+
passwordOpsLimit: keysUtils.passwordOpsLimits.min,
55+
passwordMemLimit: keysUtils.passwordMemLimits.min,
56+
strictMemoryLock: false,
57+
},
58+
},
59+
logger: logger,
60+
});
61+
});
62+
afterEach(async () => {
63+
await polykeyAgent.stop();
64+
await fs.promises.rm(dataDir, {
65+
force: true,
66+
recursive: true,
67+
});
68+
});
69+
70+
test('should edit secret', async () => {
71+
const vaultName = 'Vault10' as VaultName;
72+
const vaultId = await polykeyAgent.vaultManager.createVault(vaultName);
73+
const secretName = 'secret';
74+
75+
await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
76+
await vaultOps.addSecret(vault, secretName, 'original secret');
77+
});
78+
79+
command = ['secrets', 'edit', '-np', dataDir, `${vaultName}:${secretName}`];
80+
81+
const result = await testUtils.pkStdio([...command], {
82+
env: { PK_PASSWORD: password, EDITOR: editorEdit },
83+
cwd: dataDir,
84+
});
85+
expect(result.exitCode).toBe(0);
86+
87+
await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
88+
const contents = await vaultOps.getSecret(vault, secretName);
89+
expect(contents.toString()).toStrictEqual(`${editedContent}\n`);
90+
});
91+
});
92+
93+
test('should create secret if it does not exist', async () => {
94+
const vaultName = 'Vault10' as VaultName;
95+
const vaultId = await polykeyAgent.vaultManager.createVault(vaultName);
96+
const secretName = 'secret';
97+
98+
command = ['secrets', 'edit', '-np', dataDir, `${vaultName}:${secretName}`];
99+
100+
const result = await testUtils.pkStdio([...command], {
101+
env: { PK_PASSWORD: password, EDITOR: editorEdit },
102+
cwd: dataDir,
103+
});
104+
expect(result.exitCode).toBe(0);
105+
106+
await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
107+
const contents = await vaultOps.getSecret(vault, secretName);
108+
expect(contents.toString()).toStrictEqual(`${editedContent}\n`);
109+
});
110+
});
111+
112+
test('should not create secret if editor crashes', async () => {
113+
const vaultName = 'Vault10' as VaultName;
114+
const vaultId = await polykeyAgent.vaultManager.createVault(vaultName);
115+
const secretName = 'secret';
116+
117+
command = ['secrets', 'edit', '-np', dataDir, `${vaultName}:${secretName}`];
118+
119+
const result = await testUtils.pkStdio([...command], {
120+
env: { PK_PASSWORD: password, EDITOR: editorFail },
121+
cwd: dataDir,
122+
});
123+
expect(result.exitCode).not.toBe(0);
124+
125+
await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
126+
const list = await vaultOps.listSecrets(vault);
127+
expect(list.sort()).toStrictEqual([]);
128+
});
129+
});
130+
131+
test('should not create secret if editor does not write to file', async () => {
132+
const vaultName = 'Vault10' as VaultName;
133+
const vaultId = await polykeyAgent.vaultManager.createVault(vaultName);
134+
const secretName = 'secret';
135+
136+
command = ['secrets', 'edit', '-np', dataDir, `${vaultName}:${secretName}`];
137+
138+
const result = await testUtils.pkStdio([...command], {
139+
env: { PK_PASSWORD: password, EDITOR: editorExit },
140+
cwd: dataDir,
141+
});
142+
expect(result.exitCode).toBe(0);
143+
144+
await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
145+
const list = await vaultOps.listSecrets(vault);
146+
expect(list.sort()).toStrictEqual([]);
147+
});
148+
});
149+
150+
test('file contents should be fetched correctly', async () => {
151+
const vaultName = 'Vault10' as VaultName;
152+
const vaultId = await polykeyAgent.vaultManager.createVault(vaultName);
153+
const secretName = 'secret';
154+
const secretContent = 'original secret';
155+
156+
await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
157+
await vaultOps.addSecret(vault, secretName, secretContent);
158+
});
159+
160+
command = ['secrets', 'edit', '-np', dataDir, `${vaultName}:${secretName}`];
161+
162+
const result = await testUtils.pkStdio([...command], {
163+
env: { PK_PASSWORD: password, EDITOR: editorView },
164+
cwd: dataDir,
165+
});
166+
expect(result.exitCode).toBe(0);
167+
168+
const fetchedSecret = await fs.promises.readFile(
169+
path.join(dataDir, 'secret'),
170+
);
171+
expect(fetchedSecret.toString()).toEqual(secretContent);
172+
173+
await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
174+
const contents = await vaultOps.getSecret(vault, secretName);
175+
expect(contents.toString()).toStrictEqual(`${editedContent}\n`);
176+
});
177+
});
178+
});

‎tests/secrets/get.test.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,7 @@ describe('commandGetSecret', () => {
5252
command = ['secrets', 'get', '-np', dataDir, `${vaultName}:MySecret`];
5353

5454
const result = await testUtils.pkStdio([...command], {
55-
env: {
56-
PK_PASSWORD: password,
57-
},
55+
env: { PK_PASSWORD: password },
5856
cwd: dataDir,
5957
});
6058
expect(result.stdout).toBe('this is the secret');

‎tests/secrets/list.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ describe('commandListSecrets', () => {
8484
test(
8585
'should fail when path is not a directory',
8686
async () => {
87-
const vaultName = 'Vault5' as VaultName;
87+
const vaultName = 'Vault4' as VaultName;
8888
const vaultId = await polykeyAgent.vaultManager.createVault(vaultName);
8989

9090
await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
@@ -124,7 +124,7 @@ describe('commandListSecrets', () => {
124124
test(
125125
'should list secrets within directories',
126126
async () => {
127-
const vaultName = 'Vault6' as VaultName;
127+
const vaultName = 'Vault4' as VaultName;
128128
const vaultId = await polykeyAgent.vaultManager.createVault(vaultName);
129129

130130
await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {

0 commit comments

Comments
 (0)