Page MenuHomePhabricator

GpgCrypt::deleteHome outputs a warning on trying to delete home directory
Closed, ResolvedPublic

Description

When locally tallying an encrypted election, I found that the temporary directory storing keys etc didn't get deleted, and saw the following warnings:

Warning: unlink(/tmp/securepoll-81e957c77660234535e2a96a63886bf62a04b738/private-keys-v1.d): Is a directory [...]

Warning: rmdir(/tmp/securepoll-81e957c77660234535e2a96a63886bf62a04b738): Directory not empty [...]

GpgCrypt::deleteHome assumes that the temporary directory only contains files, and no directories:

/**
 * Delete the temporary home directory
 */
public function deleteHome() {
	if ( !$this->homeDir ) {
		return;
	}
	$dir = opendir( $this->homeDir );
	if ( !$dir ) {
		return;
	}
	// @codingStandardsIgnoreStart
	while ( false !== ( $file = readdir( $dir ) ) ) {
		// @codingStandardsIgnoreEnd
		if ( $file == '.' || $file == '..' ) {
			continue;
		}
		unlink( "$this->homeDir/$file" );
	}
	closedir( $dir );
	rmdir( $this->homeDir );
	$this->homeDir = false;
	$this->recipient = false;
}

I couldn't find similar errors on logstash over the last few months, so it may be that this assumption is correct for production (perhaps due to the particular gpg version?). However it would be useful if deleteHome could handle directories in the temporary directory.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 656494 had a related patch set uploaded (by Tchanders; owner: Tchanders):
[mediawiki/extensions/SecurePoll@master] GpgCrypt: Handle deleting directories within the home directory

https://gerrit.wikimedia.org/r/656494

Change 656494 merged by jenkins-bot:
[mediawiki/extensions/SecurePoll@master] GpgCrypt: Handle deleting directories within the home directory

https://gerrit.wikimedia.org/r/656494

@AMooney We often leave this to @Niharika, but I expect so!