From 241cfc2e83128f50bee774253300a66dfa596d00 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 21 Aug 2014 11:26:02 -0700 Subject: [PATCH] Don't leave temporary files around when trying to use credentials with destroyed secrets Summary: Ref T4284. This fixes at least one problem which can cause the observed behavior. Test Plan: - Before applying patch, used `PHABRICATOR_CREDENTIAL=PHID-CDTL-... bin/ssh-connect` + debugging prints to verify the keyfile was written and cleaned up normally. - Destroyed the credental, verified the temporary file was not cleand up correctly. - Applied patch, verified temporary file was not written and command exited with sensible error. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T4284 Differential Revision: https://secure.phabricator.com/D10328 --- .../passphrase/keys/PassphraseSSHKey.php | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/applications/passphrase/keys/PassphraseSSHKey.php b/src/applications/passphrase/keys/PassphraseSSHKey.php index 1c646704cb..0790d5342e 100644 --- a/src/applications/passphrase/keys/PassphraseSSHKey.php +++ b/src/applications/passphrase/keys/PassphraseSSHKey.php @@ -17,16 +17,21 @@ final class PassphraseSSHKey extends PassphraseAbstractKey { $file_type = PassphraseCredentialTypeSSHPrivateKeyFile::CREDENTIAL_TYPE; if ($credential->getCredentialType() != $file_type) { - // If the credential does not store a file, write the key txt out to a + // If the credential does not store a file, write the key text out to a // temporary file so we can pass it to `ssh`. if (!$this->keyFile) { + $secret = $credential->getSecret(); + if (!$secret) { + throw new Exception( + pht( + 'Attempting to use a credential ("%s") but the credential '. + 'secret has been destroyed!', + $credential->getMonogram())); + } + $temporary_file = new TempFile('passphrase-ssh-key'); - Filesystem::changePermissions($temporary_file, 0600); - - Filesystem::writeFile( - $temporary_file, - $credential->getSecret()->openEnvelope()); + Filesystem::writeFile($temporary_file, $secret->openEnvelope()); $this->keyFile = $temporary_file; }