refactor(ec2): enforce ssh key inside global storage. #5814
Conversation
Hweinstock
changed the title
| } | ||
| const mockKeys = await SshKeyPair.getSshKeyPair('fakeDir', 30000) | ||
| await client.sendSshKeyToInstance(testSelection, mockKeys, 'test-user') | ||
| const temporaryDirectory = path.join(globals.context.globalStorageUri.fsPath, 'ModelTests') |
There was a problem hiding this comment.
In tests, globalStorageUri is stubbed with a temp dir in globalSetup.test.ts, so you may be able to use the default value
But there is a chance that it is only cleared on the "after" instead of "afterEach", and may cause issues in tests:
So maybe you could add:
await fs.delete(globals.context.globalStorageUri.fsPath, { recursive: true, force: true })around here:
which I think we should have been doing in the first place
There was a problem hiding this comment.
If I move the temp keys into the globalStorage itself, then is there any reason I need this to clear on afterEach? I would think deleting the temp dir on after should be sufficient.
There was a problem hiding this comment.
If we do not clear globalStorageUri after each test we'll probably have a collision somewhere later.
Lets say you had a test that created an ssh key, then the following test assumed that since it is a new test the globalStorage will not have a key by default. The initial test state bled over in to the next and left some unexpected artifacts
There was a problem hiding this comment.
I see. Was there any reason we didn't want to do this before? Is it expensive/slow to delete all its contents for every test?
There was a problem hiding this comment.
We haven't used it much in tests before, so it was just an oversight.
| await globals.globalState.clear() | ||
|
|
||
| await testUtil.closeAllEditors() | ||
| await fs.delete(globals.context.globalStorageUri.fsPath, { recursive: true, force: true }) |
There was a problem hiding this comment.
This should be fine even for local development, because mochaGlobalSetup does:
fakeContext.globalStorageUri = (await testUtil.createTestWorkspaceFolder('globalStoragePath')).uri
3 participants
Problem
Followup from #5642 (comment)
There is a case where we could be overwriting keys/files in the
.ssh/directory if not careful.Solution
Don't allows keys to generated in a directory we do not control, and therefore avoid possibility of overwriting user ssh keys.
License: I confirm that my contribution is made under the terms of the Apache 2.0 license.