(feat): Crypto harness - #22
Merged
Merged
Conversation
Signed-off-by: Whit Waldo <whit.waldo@innovian.net>
There was a problem hiding this comment.
🔵 Needs a closer look
Unresolved moderate issues remain in test key storage and Dapr resource cleanup.
Pull request overview
Adds a Dapr cryptography harness using local-storage keys, with lifecycle helpers, tests, exports, and documentation.
Changes:
- Added local-storage cryptography component configuration.
- Added
CryptographyHarnesslifecycle and client helpers. - Added tests, public exports, and README usage documentation.
File summaries
| File | Summary |
|---|---|
src/LocalStorageCryptographyContainer.ts |
Builds the local-storage crypto component. |
src/LocalStorageCryptographyContainer.test.ts |
Tests component generation. |
src/index.ts |
Exports the new APIs. |
src/CryptographyHarness.ts |
Implements the harness; cleanup is needed for startup failures and client-stop rejection. |
src/CryptographyHarness.test.ts |
Tests configuration and encryption workflows; the key file layout requires correction. |
README.md |
Documents cryptography usage. |
Review details
Suppressed comments (3)
src/CryptographyHarness.test.ts:29
- The local-storage cryptography provider treats each file as one key, but this writes two PEM blocks (the private and public keys) into the same
testkeyfile. That is not a supported single-key file and can make the provider reject or fail to load the key during the integration test; the RSA private key already contains the public parameters needed for encryption. Write onlyprivateKey, or put the public key in a separate file with a different key name.
fs.writeFileSync(path.join(keyPath, KEY_NAME), `${privateKey}${publicKey}`, "utf8");
src/CryptographyHarness.ts:104
- When no external network is supplied, this method creates and owns a network before starting Dapr. If placement, scheduler, or sidecar startup rejects, the method exits without cleaning up the owned network (and any child containers that may already have started), leaking Docker resources on failed test setup. Add failure cleanup that can tear down partially started Dapr resources and the owned network before rethrowing.
this.daprContainer.withNetwork(this.network);
this.startedDaprContainer = await this.daprContainer.start();
src/CryptographyHarness.ts:112
- If
DaprClient.stop()rejects, this method exits before stopping the sidecar and owned network, leaving Docker resources behind.WorkflowHarness.stop()treats client shutdown as best-effort; guard this await (or otherwise use afinally) so container cleanup still runs.
if (this.daprClient) {
await this.daprClient.stop();
this.daprClient = undefined;
}
- Files reviewed: 6/6 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Whit Waldo <whit.waldo@innovian.net>
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
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.
Description
Implemented crypto harness
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #[issue number]
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: