feat(cryptography): add PGP support for encryption, decryption, signing, and verification#514
feat(cryptography): add PGP support for encryption, decryption, signing, and verification#514adrianignat13 wants to merge 9 commits intodevelopfrom
Conversation
…ng, and verification ## Problem The Cryptography activity pack only supported symmetric AES/DES/RC2/Rijndael algorithms. Users needed PGP (asymmetric, key-based) cryptography for encrypting/decrypting files and text, signing files, clear-signing files, verifying signatures, and generating PGP key pairs — all common requirements in enterprise automation workflows. ## Solution Considerations - Extend existing Encrypt/Decrypt activities with a PGP algorithm option rather than creating entirely separate activities, keeping the API surface minimal. - Create dedicated activities only for PGP-specific operations (sign, clear-sign, verify, generate key pair) that have no symmetric equivalent. - Use PgpCore NuGet package as the underlying PGP implementation. - Share signing logic via PgpFileSignHelper; manage key stream lifecycle via PgpStreamHelper. ## Actual Fix - Extended EncryptFile, DecryptFile, EncryptText, DecryptText with PGP algorithm support (public/private key + optional passphrase). - Added new activities: PgpSignFile, PgpClearSignFile, PgpVerify (unified signature + clear-signature + public-key verification), PgpGenerateKeyPair. - Added ViewModels for all new and modified activities with rule-based visibility toggling between symmetric and PGP property groups. - Added PgpStreamHelper, PgpFileSignHelper, CryptographyLocalItem model, PgpVerifyMode enum, and TranslatePgpException for user-friendly error messages. - Added comprehensive xUnit tests: PgpTests (activity-level round-trips) and PgpStandaloneTests (helper-level tests). - Registered all activities in ActivitiesMetadata.json with icons. ## Caveats - Metadata IsPrincipal mismatch on PgpVerify.Result (cosmetic, ViewModel wins at runtime). - Orphaned resource strings remain for removed intermediate verify activities (cleanup item).
Activities/Cryptography/UiPath.Cryptography.Activities.Tests/PgpStandaloneTests.cs
Fixed
Show fixed
Hide fixed
Activities/Cryptography/UiPath.Cryptography.Activities.Tests/PgpStandaloneTests.cs
Fixed
Show fixed
Hide fixed
Activities/Cryptography/UiPath.Cryptography.Activities.Tests/PgpStandaloneTests.cs
Fixed
Show fixed
Hide fixed
Activities/Cryptography/UiPath.Cryptography.Activities.Tests/PgpStandaloneTests.cs
Fixed
Show fixed
Hide fixed
Activities/Cryptography/UiPath.Cryptography.Activities.Tests/PgpStandaloneTests.cs
Fixed
Show fixed
Hide fixed
Activities/Cryptography/UiPath.Cryptography.Activities.Tests/PgpTests.cs
Fixed
Show fixed
Hide fixed
Activities/Cryptography/UiPath.Cryptography.Activities.Tests/PgpTests.cs
Fixed
Show fixed
Hide fixed
Activities/Cryptography/UiPath.Cryptography.Activities.Tests/PgpTests.cs
Fixed
Show fixed
Hide fixed
Activities/Cryptography/UiPath.Cryptography.Activities.Tests/PgpTests.cs
Fixed
Show fixed
Hide fixed
Activities/Cryptography/UiPath.Cryptography.Activities.Tests/PgpTests.cs
Fixed
Show fixed
Hide fixed
Address SonarQube S5445 findings from PR #514 — Path.GetTempFileName() is insecure (predictable names, race conditions) and can throw when >65535 temp files exist. Replaced all 19 occurrences in PgpStandaloneTests and PgpTests with Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()). Also updated code-reviewer and software-engineer agent prompts to catch this pattern and the false-positive enum rename breaking change in future reviews. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive PGP (Pretty Good Privacy) cryptography support to the UiPath Cryptography activity pack, significantly expanding its capabilities beyond symmetric encryption algorithms. The implementation integrates PgpCore 6.5.3 and adds 4 new activities while extending 4 existing activities to support PGP operations.
Changes:
- Added PGP algorithm option to existing EncryptFile, DecryptFile, EncryptText, DecryptText activities with dynamic UI toggling
- Added 4 new PGP activities: PgpGenerateKeyPair, PgpSignFile, PgpClearSignFile, and unified PgpVerify
- Renamed
SymmetricAlgorithmsenum toEncryptionAlgorithmto reflect broader scope (PGP is asymmetric) - Added helper classes for PGP stream/resource management and unified file signing logic
- Added comprehensive test coverage (PgpTests and PgpStandaloneTests)
- Fixed pre-existing bug in DecryptText.cs where ContinueOnError description pointed to Result_Description
Reviewed changes
Copilot reviewed 36 out of 42 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| UiPath.Cryptography.csproj | Added PgpCore 6.5.3 dependency and CopyLocalLockFileAssemblies flag |
| EncryptionAlgorithm.cs | Renamed from SymmetricAlgorithms.cs, added PGP enum member |
| CryptographyHelper.cs | Added PGP methods for encrypt/decrypt/sign/verify with exception translation |
| PgpStreamHelper.cs | Helper class for managing PGP key file streams and SecureString conversion |
| PgpFileSignHelper.cs | Shared logic for PgpSignFile and PgpClearSignFile activities |
| Pgp*.cs (4 activities) | New activities for key generation, signing, and verification |
| Encrypt/Decrypt*.cs (4 files) | Extended with PGP support using ViewModel-driven visibility rules |
| *ViewModel.cs (8 files) | ViewModels for all new and modified activities with dynamic property visibility |
| ActivitiesMetadata.json | Metadata entries for all PGP properties and activities |
| *.resx | Resource strings for PGP errors, modes, and property descriptions |
| Test files | Comprehensive unit and integration tests for all PGP functionality |
| Packaging | Added PgpCore and BouncyCastle DLLs to package output |
Files not reviewed (2)
- Activities/Cryptography/UiPath.Cryptography.Activities/Properties/UiPath.Cryptography.Activities.Designer.cs: Language not supported
- Activities/Cryptography/UiPath.Cryptography/Properties/UiPath.Cryptography.Designer.cs: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "DisplayNameKey": "Activity_PgpVerify_Property_Result_Name", | ||
| "TooltipKey": "Activity_PgpVerify_Property_Result_Description", | ||
| "IsRequired": false, | ||
| "IsPrincipal": true, |
There was a problem hiding this comment.
The metadata in ActivitiesMetadata.json has "IsPrincipal": true for the Result property (line 1178), but the ViewModel sets Result.IsPrincipal = false (line 56). While the ViewModel value wins at runtime, this creates an inconsistency for metadata consumers. Consider updating the JSON to match the ViewModel by setting "IsPrincipal": false for the Result property.
sorinconstantin
left a comment
There was a problem hiding this comment.
nit: LocalizedDescriptionAttribute usage on properties is redundant when view models exist. LocalizedDisplayName should also be used only for properties marked as Required, so the validation message is pointing to the correct display name of a localized property
- S2583: Replace unnecessary null-conditional telemetry calls with #if ENABLE_DEFAULT_TELEMETRY blocks in PgpSignFile, PgpClearSignFile, PgpGenerateKeyPair, PgpVerify - S3776: Reduce cognitive complexity by extracting helper methods in EncryptFile, DecryptFile, EncryptText, DecryptText - S3928: Use resource strings instead of nameof() for ArgumentNullException parameter names in PgpGenerateKeyPair, PgpVerify - S1854: Remove dead store increments in all 8 ViewModel files - S3881: Add proper IDisposable pattern in PgpStandaloneTests and PgpTests - S2325: Make GetPassphraseSecureString static in test classes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 44 changed files in this pull request and generated 11 comments.
Files not reviewed (2)
- Activities/Cryptography/UiPath.Cryptography.Activities/Properties/UiPath.Cryptography.Activities.Designer.cs: Language not supported
- Activities/Cryptography/UiPath.Cryptography/Properties/UiPath.Cryptography.Designer.cs: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Key.IsVisible = !isPgp && KeyInputModeSwitch.Value == KeyInputMode.Key; | ||
| KeySecureString.IsVisible = !isPgp && KeyInputModeSwitch.Value == KeyInputMode.SecureKey; | ||
| KeyEncodingString.IsVisible = !isPgp; | ||
|
|
There was a problem hiding this comment.
When Algorithm changes to PGP, AlgorithmChanged_Action hides Key/KeySecureString but doesn’t clear their IsRequired flags. If KeyInputModeChanged_Action previously marked one as required, this can leave hidden-but-required fields and trigger confusing design-time validation. Consider explicitly setting Key.IsRequired = false and KeySecureString.IsRequired = false whenever isPgp is true.
| // For PGP, hide key fields and ensure they are not required to avoid | |
| // hidden-but-required design-time validation issues. | |
| if (isPgp) | |
| { | |
| Key.IsRequired = false; | |
| KeySecureString.IsRequired = false; | |
| } |
|
|
||
| var encrypted = CryptographyHelper.EncryptData(Algorithm, File.ReadAllBytes(result.Item3), | ||
| CryptographyHelper.KeyEncoding(keyEncoding, key, keySecureString)); | ||
| File.WriteAllBytes(outputFilePath ?? EncryptedFile.Get(context).LocalPath, encrypted); |
There was a problem hiding this comment.
WriteEncryptedOutput already creates a CryptographyLocalItem using the (byte[], fullName, path) constructor, which writes the encrypted bytes to disk. The subsequent File.WriteAllBytes(...) writes the same content again, causing redundant I/O and potentially double-touching the output file. Consider removing the extra File.WriteAllBytes call, or alternatively write the file once and use the CryptographyLocalItem(string fullName, string existingFilePath) constructor to wrap the existing file.
| File.WriteAllBytes(outputFilePath ?? EncryptedFile.Get(context).LocalPath, encrypted); |
| public static void PgpEncryptStream(Stream inputStream, Stream outputStream, Stream publicKeyStream, Stream privateKeyStream = null, string passphrase = null, bool sign = false) | ||
| { | ||
| var shouldSign = sign && privateKeyStream != null && passphrase != null; | ||
| var encryptionKeys = shouldSign | ||
| ? new EncryptionKeys(publicKeyStream, privateKeyStream, passphrase) | ||
| : new EncryptionKeys(publicKeyStream); | ||
|
|
There was a problem hiding this comment.
shouldSign only checks passphrase != null. If callers pass an empty string, the code will attempt signing and fail with a less clear error. Consider using !string.IsNullOrEmpty(passphrase) (and similarly in PgpEncryptText) and/or explicitly throwing an ArgumentException when sign == true but the passphrase is missing/empty.
| var shouldSign = sign && privateKeyStream != null && passphrase != null; | ||
| var encryptionKeys = shouldSign | ||
| ? new EncryptionKeys(publicKeyStream, privateKeyStream, passphrase) | ||
| : new EncryptionKeys(publicKeyStream); | ||
|
|
There was a problem hiding this comment.
EncryptionKeys is constructed before the try block. If the constructor throws (e.g., invalid key stream / passphrase format), the exception won't go through TranslatePgpException and will leak a less user-friendly error. Consider moving the EncryptionKeys creation inside the try so all PGP errors get consistent translation.
| using System.ComponentModel; | ||
| using System.Diagnostics; | ||
| using System.IO; | ||
| using System.Security; |
There was a problem hiding this comment.
System.IO is imported but not used in this file, which will introduce an IDE/compiler warning. Consider removing the unused using directive.
| using System.Security; |
| if (string.IsNullOrWhiteSpace(publicKeyFilePath)) | ||
| throw new ArgumentNullException(nameof(publicKeyFilePath)); | ||
|
|
||
| using (var publicKeyStream = File.OpenRead(publicKeyFilePath)) | ||
| { | ||
| Stream privateKeyStream = null; | ||
| try | ||
| { | ||
| string passphraseString = null; | ||
| if (signData) | ||
| { | ||
| if (string.IsNullOrWhiteSpace(privateKeyFilePath)) | ||
| throw new ArgumentNullException(nameof(privateKeyFilePath)); | ||
| if (passphrase == null || passphrase.Length == 0) | ||
| throw new ArgumentNullException(nameof(passphrase)); | ||
| passphraseString = new NetworkCredential("", passphrase).Password; | ||
| privateKeyStream = File.OpenRead(privateKeyFilePath); | ||
| } |
There was a problem hiding this comment.
WithPgpEncryptStreams opens the key files directly. When the path is non-empty but the file is missing, File.OpenRead will throw FileNotFoundException with a non-localized message. Consider adding File.Exists checks (for the public key and, when signing, the private key) and throwing an ArgumentException using the existing localized resource strings (e.g., FileDoesNotExistsException) so activity errors are consistent and user-friendly.
| if (string.IsNullOrWhiteSpace(privateKeyFilePath)) | ||
| throw new ArgumentNullException(nameof(privateKeyFilePath)); | ||
| if (passphrase == null || passphrase.Length == 0) | ||
| throw new ArgumentNullException(nameof(passphrase)); | ||
|
|
||
| var passphraseString = new NetworkCredential("", passphrase).Password; | ||
|
|
||
| using (var privateKeyStream = File.OpenRead(privateKeyFilePath)) | ||
| { | ||
| Stream publicKeyStream = null; | ||
| try | ||
| { | ||
| if (verifySignature) | ||
| { | ||
| if (string.IsNullOrWhiteSpace(publicKeyFilePath)) | ||
| throw new ArgumentNullException(nameof(publicKeyFilePath)); | ||
| publicKeyStream = File.OpenRead(publicKeyFilePath); | ||
| } | ||
|
|
There was a problem hiding this comment.
WithPgpDecryptStreams opens the private/public key files directly. If the file path is provided but the file doesn't exist, File.OpenRead throws FileNotFoundException (non-localized) instead of the pack’s usual FileDoesNotExistsException messaging. Consider adding explicit File.Exists checks for the private key and, when verifySignature == true, the public key and throwing a localized ArgumentException.
| decrypted = CryptographyHelper.DecryptData(Algorithm, encrypted, CryptographyHelper.KeyEncoding(keyEncoding, key, keySecureString)); | ||
| } | ||
| catch (CryptographicException ex) | ||
| File.WriteAllBytes(outputFilePath ?? DecryptedFile.Get(context).LocalPath, decrypted); |
There was a problem hiding this comment.
WriteDecryptedOutput constructs a CryptographyLocalItem via the (byte[], fullName, path) constructor which already writes the decrypted bytes to disk. The following File.WriteAllBytes(...) writes the same bytes again, duplicating I/O and risking surprising overwrite semantics. Consider removing the extra write, or write once and wrap the file using the CryptographyLocalItem(string fullName, string existingFilePath) constructor.
| File.WriteAllBytes(outputFilePath ?? DecryptedFile.Get(context).LocalPath, decrypted); |
| catch (Exception ex) | ||
| { | ||
| throw TranslatePgpException(ex); | ||
| } |
There was a problem hiding this comment.
In the PGP wrappers, throw TranslatePgpException(ex); rethrows a (potentially unchanged) exception instance which resets the original stack trace, making failures harder to diagnose. Consider either always wrapping into a new exception type (preserving ex as InnerException), or if no translation is performed rethrow with throw; / ExceptionDispatchInfo.Capture(ex).Throw() to preserve the original stack.
| UpdateDeprecatedAlgorithmWarning(); | ||
|
|
||
| bool isPgp = Algorithm.Value == EncryptionAlgorithm.PGP; | ||
|
|
There was a problem hiding this comment.
When switching Algorithm to PGP, AlgorithmChanged_Action hides Key/KeySecureString but doesn’t clear their IsRequired flags. If the user previously had a symmetric algorithm selected, KeyInputModeChanged_Action may have marked one of these as required, which can leave a hidden-but-required field and cause confusing design-time validation. Consider explicitly setting Key.IsRequired = false and KeySecureString.IsRequired = false when isPgp is true (and similarly reset other symmetric-only required flags).
| if (isPgp) | |
| { | |
| // When switching to PGP, symmetric key fields are hidden, so they must not remain required. | |
| Key.IsRequired = false; | |
| KeySecureString.IsRequired = false; | |
| KeyEncodingString.IsRequired = false; | |
| } |
- Remove duplicate File.WriteAllBytes in EncryptFile/DecryptFile (CryptographyLocalItem constructor already writes) - Clear Key/KeySecureString IsRequired flags when switching to PGP algorithm in both Encrypt/Decrypt ViewModels - Move EncryptionKeys construction inside try blocks for consistent error translation - Use string.IsNullOrEmpty for passphrase checks instead of null-only - Preserve stack traces via ExceptionDispatchInfo when PGP exceptions are not translated - Add File.Exists validation with localized errors in PgpStreamHelper before File.OpenRead - Remove unused using System.IO from EncryptText.cs and DecryptText.cs - Fix IsPrincipal metadata inconsistency for Result in EncryptText/DecryptText - Remove redundant LocalizedDisplayName/LocalizedDescription from non-required PGP properties Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…elds Set IsRequired alongside IsVisible using the same condition expression, making property state easier to reason about at a glance. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 44 changed files in this pull request and generated no new comments.
Files not reviewed (2)
- Activities/Cryptography/UiPath.Cryptography.Activities/Properties/UiPath.Cryptography.Activities.Designer.cs: Language not supported
- Activities/Cryptography/UiPath.Cryptography/Properties/UiPath.Cryptography.Designer.cs: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 44 changed files in this pull request and generated 6 comments.
Files not reviewed (2)
- Activities/Cryptography/UiPath.Cryptography.Activities/Properties/UiPath.Cryptography.Activities.Designer.cs: Language not supported
- Activities/Cryptography/UiPath.Cryptography/Properties/UiPath.Cryptography.Designer.cs: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var shouldSign = sign && privateKeyStream != null && !string.IsNullOrEmpty(passphrase); | ||
| var encryptionKeys = shouldSign | ||
| ? new EncryptionKeys(publicKeyStream, privateKeyStream, passphrase) | ||
| : new EncryptionKeys(publicKeyStream); | ||
|
|
There was a problem hiding this comment.
PgpEncryptStream silently falls back to unencrypted-only behavior when sign=true but privateKeyStream/passphrase are missing (shouldSign becomes false). This can lead to callers believing data was signed when it wasn’t. Consider validating inputs and throwing an ArgumentException/ArgumentNullException when sign==true but required signing inputs are not provided.
| var shouldVerify = verifySignature && publicKeyStream != null; | ||
| var encryptionKeys = shouldVerify | ||
| ? new EncryptionKeys(publicKeyStream, privateKeyStream, passphrase) | ||
| : new EncryptionKeys(privateKeyStream, passphrase); | ||
|
|
There was a problem hiding this comment.
PgpDecryptStream silently decrypts without verification when verifySignature=true but publicKeyStream is null (shouldVerify becomes false). This can cause a security footgun where signature verification is requested but not actually performed. Consider enforcing that publicKeyStream must be provided when verifySignature==true (throw an argument exception) instead of silently disabling verification.
|


Problem
The Cryptography activity pack only supported symmetric algorithms (AES, DES, RC2, Rijndael). Users needed PGP (asymmetric, key-based) cryptography for encrypting/decrypting files and text,
signing files, clear-signing files, verifying signatures, and generating key pairs — common requirements in enterprise automation workflows.
Solution
visibility toggling.
ViewModels), PgpSignViewModelBase (shared layout for sign/clear-sign ViewModels).
43 files changed — 4,655 insertions, 936 deletions.
Review Results