handle malformed PGP keys without crashing (fix #715) - #726
Merged
Conversation
ObjectivePGP raises an Objective-C exception when reading an armored block without a body. Swift cannot catch it, so the app terminated as soon as a key ID was requested, e.g. when opening the settings screen. As the keys live in the keychain, they survive reinstalling the app and the crash persisted. Route the calls into ObjectivePGP through an Objective-C exception catcher which turns exceptions into regular errors. Additionally, offer the removal of PGP keys whenever keys are stored, not only when a key source is set. The key source is gone after a reinstall while the keys are not, which left no way to get rid of unusable keys. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #715.
Root cause
ObjectivePGP.readKeys(from:)raises an Objective-C exception (NSInvalidArgumentException: -[NSPlaceholderMutableString initWithString:]: nil argument) when it is given an armored block with headers but no body. Swift cannot catch anNSException, so the process aborts.PGPAgent.initKeys()falls back to ObjectivePGP whenever GopenPGP rejects a key, and the settings screen requests the short key ID inviewDidLoadjust to render the detail label. With such a key stored, opening the settings screen was an immediate crash. The keys live in the keychain and therefore survive uninstalling the app, which is why reinstalling did not help.Of eight malformed key variants tested against
PGPAgent, only the empty armor block crashed; garbage base64, truncated keys, dropped characters and plain text already produced proper errors.A second problem prevented recovery: the "Remove PGP Keys" action was only offered when
Defaults.pgpKeySource != nil. That default lives in the user defaults and is wiped by an uninstall, so after reinstalling the option was hidden while the broken key was still present.Changes
ObjectiveCExceptionCatcherin passKit which runs a block inside@try/@catchand converts anNSExceptioninto anNSError.ObjectivePGPInterfaceroutes key reading, decryption and encryption through it and reportsAppError.keyImport,.decryptionand.encryptioninstead of terminating. The user now sees "Cannot import the key.".Testing
bundle exec fastlane testequivalent run locally: 145 tests, 0 failures. The newtestEmptyArmoredKeyfails without the fix.🤖 Generated with Claude Code