Skip to content

(feat): Crypto harness - #22

Merged
WhitWaldo merged 5 commits into
mainfrom
crypto-harness
Sep 15, 2026
Merged

WhitWaldo merged 5 commits into
mainfrom
crypto-harness

Conversation

@WhitWaldo

Copy link
Copy Markdown
Contributor

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:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

Signed-off-by: Whit Waldo <whit.waldo@innovian.net>
@WhitWaldo
WhitWaldo requested a lite review from Copilot September 14, 2026 01:01
@WhitWaldo WhitWaldo self-assigned this Sep 14, 2026
@WhitWaldo WhitWaldo added the enhancement New feature or request label Sep 14, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 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 CryptographyHarness lifecycle 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 testkey file. 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 only privateKey, 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 a finally) 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.

WhitWaldo and others added 4 commits September 15, 2026 01:56
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>
@WhitWaldo
WhitWaldo merged commit 5ad2a07 into main Sep 15, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants