feature/webhook_plugin_signing #2703
Conversation
|
WalkthroughThe changes introduced in this pull request primarily focus on updating various dependencies across multiple packages within the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
packages/common/src/utils/sign/sign.ts (1)
3-9: LGTM with a minor suggestion!The
signfunction correctly implements the HMAC-SHA256 signing of the payload using the provided key. It also handles the case when the key is not provided by returning 'UNSIGNED'.Consider changing the
payloadtype to a more specific type (e.g.,Record<string, unknown>) or adding type checks to ensure the payload is serializable before stringifying it. This can help catch potential issues early and improve the function's type safety.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (32)
- apps/backoffice-v2/CHANGELOG.md (1 hunks)
- apps/backoffice-v2/package.json (2 hunks)
- apps/kyb-app/CHANGELOG.md (1 hunks)
- apps/kyb-app/package.json (2 hunks)
- examples/headless-example/CHANGELOG.md (1 hunks)
- examples/headless-example/package.json (2 hunks)
- packages/common/CHANGELOG.md (1 hunks)
- packages/common/package.json (2 hunks)
- packages/common/src/index.ts (1 hunks)
- packages/common/src/utils/index.ts (1 hunks)
- packages/common/src/utils/sign/index.ts (1 hunks)
- packages/common/src/utils/sign/sign.ts (1 hunks)
- packages/common/src/utils/sign/sign.unit.test.ts (4 hunks)
- packages/workflow-core/CHANGELOG.md (1 hunks)
- packages/workflow-core/package.json (2 hunks)
- packages/workflow-core/src/lib/plugins/external-plugin/webhook-plugin.ts (3 hunks)
- sdks/web-ui-sdk/CHANGELOG.md (1 hunks)
- sdks/web-ui-sdk/package.json (2 hunks)
- sdks/workflow-browser-sdk/CHANGELOG.md (1 hunks)
- sdks/workflow-browser-sdk/package.json (2 hunks)
- sdks/workflow-node-sdk/CHANGELOG.md (1 hunks)
- sdks/workflow-node-sdk/package.json (2 hunks)
- services/workflows-service/CHANGELOG.md (1 hunks)
- services/workflows-service/package.json (2 hunks)
- services/workflows-service/scripts/seed.ts (1 hunks)
- services/workflows-service/src/alert/alert.service.ts (1 hunks)
- services/workflows-service/src/alert/webhook-manager/webhook-manager.service.ts (1 hunks)
- services/workflows-service/src/common/utils/sign/sign.ts (0 hunks)
- services/workflows-service/src/events/document-changed-webhook-caller.ts (1 hunks)
- services/workflows-service/src/events/workflow-completed-webhook-caller.ts (1 hunks)
- services/workflows-service/src/events/workflow-state-changed-webhook-caller.ts (1 hunks)
- websites/docs/package.json (1 hunks)
Files not reviewed due to no reviewable changes (1)
- services/workflows-service/src/common/utils/sign/sign.ts
Files skipped from review due to trivial changes (19)
- apps/backoffice-v2/CHANGELOG.md
- apps/kyb-app/CHANGELOG.md
- apps/kyb-app/package.json
- examples/headless-example/CHANGELOG.md
- examples/headless-example/package.json
- packages/common/src/utils/sign/index.ts
- packages/common/src/utils/sign/sign.unit.test.ts
- packages/workflow-core/CHANGELOG.md
- packages/workflow-core/package.json
- sdks/web-ui-sdk/CHANGELOG.md
- sdks/web-ui-sdk/package.json
- sdks/workflow-browser-sdk/CHANGELOG.md
- sdks/workflow-browser-sdk/package.json
- sdks/workflow-node-sdk/CHANGELOG.md
- services/workflows-service/CHANGELOG.md
- services/workflows-service/src/events/document-changed-webhook-caller.ts
- services/workflows-service/src/events/workflow-completed-webhook-caller.ts
- services/workflows-service/src/events/workflow-state-changed-webhook-caller.ts
- websites/docs/package.json
Additional comments not posted (17)
packages/common/src/index.ts (1)
33-34: LGTM!The addition of the
signandcomputeHashexports from the./utilsmodule is a good enhancement to provide cryptographic capabilities for signing data and generating hash values. This change is consistent with the AI-generated summary and the exports are correctly declared.packages/common/src/utils/index.ts (1)
31-31: LGTM!The addition of the
signandcomputeHashexports from the./signmodule is a good change that expands the utility functions available for use in other parts of the application. These functions are likely to be useful for implementing security features related to the shared webhook functionality, such as signing webhook payloads for authentication and integrity checks, and computing hashes for indexing and other purposes.The exports are correctly declared and follow the existing pattern in the file.
services/workflows-service/src/alert/webhook-manager/webhook-manager.service.ts (1)
12-12: Verify the behavior of thesignfunction from the new import path.The import path change for the
signfunction looks good. It aligns with the refactor to consolidate cryptographic functionalities within the@ballerine/commonpackage.To ensure the
signfunction from the new import path behaves correctly, please run the following script:Please let me know if the script yields the expected output. This will confirm the
signfunction from the new import path is being used correctly to generate the webhook signature.Verification successful
Verification successful: The
signfunction is used correctly from the new import path.The
signfunction is correctly utilized within thesendWebhookmethod with the expected arguments (payload: dataandkey: webhookSharedSecret). The import path change aligns with the refactor and does not affect the functionality.
- Location:
services/workflows-service/src/alert/webhook-manager/webhook-manager.service.tsat lines 56-59.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the `sign` function from the new import path generates the expected signature. # Test: Search for the `sign` function usage within the `sendWebhook` method. ast-grep --lang typescript --pattern $'sendWebhook($_) { $$$ sign({ payload: $_, key: $_, }) $$$ }' # Expected output: The `sign` function should be called with the `data` and `webhookSharedSecret` arguments to generate the `X-HMAC-Signature` header.Length of output: 614
Script:
#!/bin/bash # Description: Verify the `sign` function usage within the `sendWebhook` method. # Step 1: Locate the `sendWebhook` method in the codebase. ast-grep --lang typescript --pattern 'function sendWebhook($$$) { $$$ }' -A 10 # Step 2: Search for the `sign` function usage in the located method. ast-grep --lang typescript --pattern 'sign({ payload: $_, key: $_ })' -A 2Length of output: 804
sdks/workflow-node-sdk/package.json (1)
4-4: LGTM!The version updates for the
@ballerine/workflow-node-sdkpackage and its@ballerine/workflow-coredependency are consistent and follow semantic versioning. The minor version increment indicates potential bug fixes, enhancements, or new features without introducing any breaking changes. These updates are necessary to ensure compatibility and reflect the latest changes in the package and its dependency.Also applies to: 31-31
packages/common/package.json (3)
5-5: LGTM!The version update follows the semantic versioning format and aligns with the PR objective of adding a new webhook feature.
79-79: LGTM!Adding
vitestas a dev dependency is a good practice for writing and running tests. The version constraint^0.28.4allows for minor and patch updates while ensuring compatibility.
80-80: Verify the usage ofcrypto-jsin the codebase.The addition of
crypto-jsand@types/crypto-jsdependencies suggests that the package will be using cryptographic functionality, which aligns with the AI-generated summary. The version constraints allow for minor and patch updates while ensuring compatibility.To verify the usage of
crypto-jsin the codebase, run the following script:Also applies to: 85-85
Verification successful
Verification successful:
crypto-jsis used in the codebase.The
crypto-jslibrary is imported in the filepackages/common/src/utils/sign/sign.ts, confirming its usage. This aligns with the addition of thecrypto-jsand@types/crypto-jsdependencies.
packages/common/src/utils/sign/sign.ts:import CryptoJS from 'crypto-js';Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `crypto-js` in the codebase. # Test: Search for the import statements of `crypto-js`. Expect: At least one occurrence. rg --type typescript $'import.*crypto-js'Length of output: 77
Script:
#!/bin/bash # Description: Verify the usage of `crypto-js` in the codebase using ast-grep. # Test: Search for import statements of `crypto-js` in TypeScript files. ast-grep --lang typescript --pattern $'import $_ from "crypto-js"'Length of output: 67
Script:
#!/bin/bash # Description: Verify the usage of `crypto-js` in the codebase by searching for import statements in both JavaScript and TypeScript files. # Test: Search for import statements of `crypto-js` in JavaScript and TypeScript files. rg 'import.*crypto-js' --type js --type tsLength of output: 117
packages/workflow-core/src/lib/plugins/external-plugin/webhook-plugin.ts (2)
33-33: LGTM!The method call to
composeRequestSignedHeadersis correctly awaited and passes the expected arguments. The changes are consistent with the list of alterations provided in the summary.
79-105: Excellent work on enhancing the security and flexibility of the WebhookPlugin!The new
composeRequestSignedHeadersmethod is a great addition to theWebhookPluginclass. It enables dynamic header composition based on external secrets and context, which significantly improves the security and flexibility of the plugin.The method implementation is correct and matches the description provided in the summary. The use of the
signfunction from@ballerine/commonto generate theX-HMAC-Signatureheader is a good practice for ensuring the integrity of the request payload. The asynchronous processing of header values usingPromise.allandreplaceValuePlaceholdersis an efficient way to resolve all header values before returning the final headers object.Overall, the changes are well-implemented and enhance the functionality of the
WebhookPlugin. Great job!apps/backoffice-v2/package.json (1)
3-3: LGTM! The version updates are appropriate and align with the changes in the@ballerine/commonpackage.The changes in this
package.jsonfile are primarily focused on version increments for the@ballerine/backoffice-v2package and its dependencies. Based on the AI-generated summary, the@ballerine/commonpackage has been enhanced with new cryptographic functionalities, including signing logic. The version updates for the@ballerine/workflow-browser-sdkand@ballerine/workflow-node-sdkpackages are likely related to the changes in the@ballerine/commonpackage to ensure compatibility and potentially improve security and performance.These updates are appropriate and necessary to keep the project in sync with the latest changes in its dependencies.
Also applies to: 54-54, 57-58
services/workflows-service/package.json (4)
4-4: LGTM!The version update follows semantic versioning and indicates a minor release, which is expected to introduce new features, improvements, or bug fixes without breaking changes.
50-50: Verify compatibility with the updated@ballerine/commonpackage.Updating the
@ballerine/commondependency to the latest version is a good practice to ensure access to the latest features, improvements, and bug fixes. However, it's important to verify that the update does not introduce any breaking changes that could affect the@ballerine/workflows-servicepackage.Please review the changelog or release notes of the
@ballerine/commonpackage for version0.9.29to ensure compatibility and the absence of any breaking changes.
51-51: Verify compatibility with the updated@ballerine/workflow-corepackage.Updating the
@ballerine/workflow-coredependency to the latest version is a good practice to ensure access to the latest features, improvements, and bug fixes related to the core workflow functionality. However, it's important to verify that the update does not introduce any breaking changes that could affect the@ballerine/workflows-servicepackage.Please review the changelog or release notes of the
@ballerine/workflow-corepackage for version0.6.40to ensure compatibility and the absence of any breaking changes.
52-52: Verify compatibility with the updated@ballerine/workflow-node-sdkpackage.Updating the
@ballerine/workflow-node-sdkdependency to the latest version is a good practice to ensure access to the latest features, improvements, and bug fixes related to the Node.js SDK for workflows. However, it's important to verify that the update does not introduce any breaking changes that could affect the@ballerine/workflows-servicepackage.Please review the changelog or release notes of the
@ballerine/workflow-node-sdkpackage for version0.6.40to ensure compatibility and the absence of any breaking changes.packages/common/CHANGELOG.md (1)
3-7: LGTM!The changelog entry is well-formatted, follows the existing structure, and accurately describes the changes made in version 0.9.29. The addition of signing logic using the
cryptlibrary is a significant change that justifies the patch version bump.services/workflows-service/src/alert/alert.service.ts (1)
25-25: Verify the compatibility of the newcomputeHashimplementation.The import statement for
computeHashhas been updated to import from@ballerine/common. This change indicates a refactoring or reorganization of the codebase. While the functionality of the service may remain unchanged if the implementation ofcomputeHashis the same in both locations, it's important to verify that the new implementation is compatible and does not introduce any breaking changes.Run the following script to verify the compatibility of the new
computeHashimplementation:Verification successful
Verification successful:
computeHashimplementation is compatible.The
computeHashfunction is now imported from@ballerine/common, and its implementation inpackages/common/src/utils/sign/sign.tsis consistent with its usage in the alert service. The refactor appears to be organizational, with no breaking changes introduced.
computeHashimplementation usesCryptoJS.MD5for hashing, which aligns with typical usage patterns.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the compatibility of the new `computeHash` implementation. # Test 1: Search for the `computeHash` usage in the codebase. # Expect: The function should be used in a similar manner as before. rg --type typescript -A 5 $'computeHash' # Test 2: Compare the implementation of `computeHash` in `@ballerine/common` with the old implementation in `@/common/utils/sign/sign`. # Expect: The implementations should be similar or equivalent. diff <(fd -e ts computeHash.ts /path/to/old/implementation) <(fd -e ts computeHash.ts /path/to/new/implementation)Length of output: 531
Script:
#!/bin/bash # Description: Verify the compatibility of the new `computeHash` implementation. # Test 1: Search for the `computeHash` usage in the codebase. rg -A 5 'computeHash' # Test 2: Locate the files containing `computeHash` in the repository. fd -e ts 'computeHash.ts' --exec echo "Found: {}" # Test 3: Compare the implementation of `computeHash` in the identified files. # Note: This assumes that the paths to the old and new implementations are found in the previous step. # Replace <old_path> and <new_path> with actual paths once identified. # diff <old_path> <new_path>Length of output: 1900
services/workflows-service/scripts/seed.ts (1)
478-478: Theexvariable is undefined. Verify its purpose and definition.The code change adds an
exproperty to the state object, but theexvariable is not defined anywhere in the provided code context. This will result in a runtime error.Please verify:
- Where is the
exvariable defined?- What is the purpose of adding it to the state object?
To search for the definition of the
exvariable in the codebase, run:
| export const computeHash = (data: unknown): string => { | ||
| const md5hash = CryptoJS.MD5(JSON.stringify(data)); | ||
|
|
||
| return md5hash.toString(CryptoJS.enc.Hex); | ||
| }; |
There was a problem hiding this comment.
Use a more secure hash function and consider improving type safety.
The computeHash function correctly computes the hash of the input data and returns it as a hexadecimal string. However, there are two issues to consider:
-
The function uses the MD5 hash function, which is known to have security vulnerabilities and is not recommended for use in new cryptographic protocols. Consider using a more secure hash function like SHA-256.
-
The
dataparameter is of typeunknown, which allows any type of data to be passed as input. This could potentially lead to issues if the data is not serializable. Consider changing thedatatype to a more specific type (e.g.,Record<string, unknown>) or adding type checks to ensure the data is serializable before stringifying it. This can help catch potential issues early and improve the function's type safety.
Apply this diff to address the issues:
-export const computeHash = (data: unknown): string => {
- const md5hash = CryptoJS.MD5(JSON.stringify(data));
+export const computeHash = (data: Record<string, unknown>): string => {
+ const sha256hash = CryptoJS.SHA256(JSON.stringify(data));
- return md5hash.toString(CryptoJS.enc.Hex);
+ return sha256hash.toString(CryptoJS.enc.Hex);
};Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const computeHash = (data: unknown): string => { | |
| const md5hash = CryptoJS.MD5(JSON.stringify(data)); | |
| return md5hash.toString(CryptoJS.enc.Hex); | |
| }; | |
| export const computeHash = (data: Record<string, unknown>): string => { | |
| const sha256hash = CryptoJS.SHA256(JSON.stringify(data)); | |
| return sha256hash.toString(CryptoJS.enc.Hex); | |
| }; |
# Conflicts: # apps/backoffice-v2/package.json # apps/kyb-app/CHANGELOG.md # apps/kyb-app/package.json # services/workflows-service/prisma/data-migrations
packages/workflow-core/src/lib/plugins/external-plugin/webhook-plugin.ts
Outdated
Show resolved
Hide resolved
# Conflicts: # apps/backoffice-v2/CHANGELOG.md # examples/headless-example/CHANGELOG.md # examples/headless-example/package.json # packages/common/CHANGELOG.md # packages/common/package.json # packages/workflow-core/CHANGELOG.md # packages/workflow-core/package.json # pnpm-lock.yaml # sdks/web-ui-sdk/CHANGELOG.md # sdks/web-ui-sdk/package.json # sdks/workflow-browser-sdk/CHANGELOG.md # sdks/workflow-browser-sdk/package.json # sdks/workflow-node-sdk/CHANGELOG.md # sdks/workflow-node-sdk/package.json # services/workflows-service/CHANGELOG.md # services/workflows-service/package.json # services/workflows-service/prisma/data-migrations # websites/docs/package.json
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (3)
packages/workflow-core/CHANGELOG.md (1)
3-7: LGTM with a minor suggestion.The new changelog entry for version 0.6.45 is correctly added and properly classified as a patch change. The description aligns with the PR objectives of adding webhook functionality.
Consider updating the description to fix a minor typo:
- - Added webhook signin + - Added webhook signingapps/kyb-app/CHANGELOG.md (1)
3-7: LGTM! Consider adding a brief description of the changes.The new version entry (0.3.58) is correctly formatted and includes the necessary information about the updated dependency. To improve clarity for users, consider adding a brief description of the changes or improvements introduced by this update.
You could add a short description like this:
## 0.3.58 ### Patch Changes +- Updated dependency @ballerine/workflow-browser-sdk to version 0.6.45 - @ballerine/workflow-browser-sdk@0.6.45apps/backoffice-v2/CHANGELOG.md (1)
3-8: LGTM! Consider adding a brief description of the changes.The changelog entry for version 0.7.48 correctly documents the patch changes for the workflow SDKs. The version numbers are consistent, and the changes align with the AI-generated summary.
Consider adding a brief description of the changes or the reason for the dependency updates. This can help users understand the impact of the update. For example:
## 0.7.48 ### Patch Changes + +- Updated workflow SDKs to improve performance and fix minor bugs + - @ballerine/workflow-browser-sdk@0.6.45 - @ballerine/workflow-node-sdk@0.6.45
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
- apps/backoffice-v2/CHANGELOG.md (1 hunks)
- apps/backoffice-v2/package.json (2 hunks)
- apps/kyb-app/CHANGELOG.md (1 hunks)
- apps/kyb-app/package.json (2 hunks)
- examples/headless-example/CHANGELOG.md (1 hunks)
- examples/headless-example/package.json (2 hunks)
- packages/workflow-core/CHANGELOG.md (1 hunks)
- packages/workflow-core/package.json (1 hunks)
- sdks/workflow-browser-sdk/CHANGELOG.md (1 hunks)
- sdks/workflow-browser-sdk/package.json (2 hunks)
- sdks/workflow-node-sdk/CHANGELOG.md (1 hunks)
- sdks/workflow-node-sdk/package.json (2 hunks)
- services/workflows-service/CHANGELOG.md (1 hunks)
- services/workflows-service/package.json (2 hunks)
- services/workflows-service/src/workflow/workflow.service.ts (5 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/workflow-core/package.json
- sdks/workflow-browser-sdk/CHANGELOG.md
- services/workflows-service/CHANGELOG.md
🔇 Additional comments (17)
sdks/workflow-node-sdk/package.json (2)
4-4: LGTM: Package version updateThe package version has been correctly incremented from 0.6.44 to 0.6.45. This minor version bump aligns with the addition of the new webhook feature mentioned in the PR objectives.
31-31: LGTM: Dependency version updateThe @ballerine/workflow-core dependency has been correctly updated to version 0.6.45, matching the package version update. This alignment ensures compatibility between the packages.
To ensure consistency across the project, please run the following script to verify that all references to @ballerine/workflow-core are using the updated version:
✅ Verification successful
<!-- [monologue] [task verify_dependency_update_2/2] The latest shell script output shows that all instances of "@ballerine/workflow-core" in the dependency sections of package.json files are either set to "0.6.45" or are not present. There are no mismatches or incorrect versions found. [observations] - All dependencies referencing "@ballerine/workflow-core" are correctly using version "0.6.45". - The presence of "null" indicates that some package.json files do not include this dependency, which is acceptable if not required. [analysis] - Since no inconsistencies are found in the dependency versions, the initial review comment is validated. - No further verification scripts are necessary. [actions] - Conclude the verification process and generate the final approval response with the tag. -->LGTM: Dependency version update
All references to
@ballerine/workflow-corehave been correctly updated to version0.6.45across the project. This ensures consistent compatibility throughout.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to @ballerine/workflow-core are using version 0.6.45 # Test: Search for @ballerine/workflow-core in all package.json files # Expect: All occurrences should use version 0.6.45 fd package.json --exec rg --json '"@ballerine/workflow-core"' {} | jq -r '.data.lines.text' | grep -v '"@ballerine/workflow-core": "0.6.45"' # If the above command returns any results, it indicates inconsistencies that need to be addressed.Length of output: 344
Script:
#!/bin/bash # Description: Verify that all dependencies to @ballerine/workflow-core are using version 0.6.45 # Search all package.json files and check dependencies fd package.json --exec jq -r ' .dependencies["@ballerine/workflow-core"], .devDependencies["@ballerine/workflow-core"], .peerDependencies["@ballerine/workflow-core"], .optionalDependencies["@ballerine/workflow-core"] ' {} | grep -v '^"0\.6\.45"$'Length of output: 744
sdks/workflow-browser-sdk/package.json (1)
4-4: LGTM: Version update is consistent and aligned with dependencies.The package version has been correctly bumped from 0.6.44 to 0.6.45, and the
@ballerine/workflow-coredependency has been updated to match this new version. This consistency is good practice and helps maintain compatibility between related packages.To ensure version consistency across related packages, please run the following script:
This script will help verify that the version update has been applied consistently across related packages in the @ballerine ecosystem.
Also applies to: 37-37
✅ Verification successful
Verification Successful: Version consistency confirmed across related packages.
All package versions and their
@ballerine/workflow-coredependencies have been correctly updated to 0.6.45 across theworkflow-browser-sdk,workflow-node-sdk, andworkflow-corepackages. This consistency ensures compatibility within the @ballerine ecosystem.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify version consistency across related @ballerine packages # Test: Check versions of related packages echo "Checking versions of related @ballerine packages:" jq -r '.version' sdks/workflow-browser-sdk/package.json jq -r '.version' sdks/workflow-node-sdk/package.json jq -r '.version' packages/workflow-core/package.json # Test: Check @ballerine/workflow-core dependency version in related packages echo "Checking @ballerine/workflow-core dependency version in related packages:" jq -r '.dependencies."@ballerine/workflow-core"' sdks/workflow-browser-sdk/package.json jq -r '.dependencies."@ballerine/workflow-core"' sdks/workflow-node-sdk/package.jsonLength of output: 639
apps/kyb-app/package.json (2)
4-4: Version bump looks good.The version has been incremented from 0.3.57 to 0.3.58, which is consistent with a patch update. This change aligns with the PR objectives and the AI-generated summary.
20-20: Dependency update is appropriate.The @ballerine/workflow-browser-sdk dependency has been updated from version 0.6.44 to 0.6.45. This patch update is in line with the changes described in the AI-generated summary.
apps/backoffice-v2/package.json (4)
3-3: LGTM: Version bump is appropriate.The version update from 0.7.47 to 0.7.48 follows semantic versioning principles, indicating a minor update with new features or non-breaking changes.
57-57: LGTM: Dependency update is appropriate.The @ballerine/workflow-browser-sdk dependency has been updated from 0.6.44 to 0.6.45. This patch version update likely includes bug fixes or minor improvements and should be compatible with the project.
58-58: LGTM: Dependency update is appropriate and consistent.The @ballerine/workflow-node-sdk dependency has been updated from 0.6.44 to 0.6.45, matching the version update of @ballerine/workflow-browser-sdk. This consistency in versioning is good practice and should help maintain compatibility across the project.
3-3: Summary: Package updates are consistent and appropriate.The changes in this file include:
- A minor version bump of the package itself (0.7.47 to 0.7.48).
- Patch updates to two related dependencies: @ballerine/workflow-browser-sdk and @ballerine/workflow-node-sdk (both from 0.6.44 to 0.6.45).
These updates are consistent with each other and follow semantic versioning principles. They likely represent coordinated improvements across the Ballerine ecosystem. The changes appear to be part of the "feature/webhook_plugin_signing" PR, which aims to add a webhook implementing shared webhook functionality.
To ensure the stability of the system after these updates:
Run the following script to check for any breaking changes or deprecations in the updated packages:
This script will help verify that the updates don't introduce any unexpected breaking changes and that the package still builds and passes tests with the new dependencies.
Also applies to: 57-58
services/workflows-service/package.json (2)
4-4: LGTM: Version bump is appropriate.The version increment from 0.7.47 to 0.7.48 aligns with the addition of the new webhook feature mentioned in the PR objectives. This minor version bump correctly indicates non-breaking changes or new features.
51-52: Dependency updates look good, but please verify.The updates to
@ballerine/workflow-coreand@ballerine/workflow-node-sdk(both to version 0.6.45) are consistent and appear to be minor version bumps. This is generally good practice. However:
- Please confirm that these updates are necessary for the new webhook feature.
- Ensure that you've tested the compatibility of these new versions with your changes.
To help verify the necessity of these updates, you can run the following script:
This script will help you confirm if these dependencies are being used in the new webhook feature or in any recently modified files.
✅ Verification successful
Dependency updates verified and approved.
The updates to
@ballerine/workflow-coreand@ballerine/workflow-node-sdk(both to version 0.6.45) have been confirmed through their extensive usage across the codebase. These minor version increments are appropriate and should support the new webhook feature effectively.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the updated dependencies are used in the new webhook feature # Search for usage of @ballerine/workflow-core and @ballerine/workflow-node-sdk echo "Searching for @ballerine/workflow-core usage:" rg --type typescript '@ballerine/workflow-core' echo "\nSearching for @ballerine/workflow-node-sdk usage:" rg --type typescript '@ballerine/workflow-node-sdk' # Check for any new imports or usage in recently modified files echo "\nChecking for new imports in recently modified files:" git diff --name-only HEAD~1 | xargs rg --type typescript -e '@ballerine/workflow-core' -e '@ballerine/workflow-node-sdk'Length of output: 656
Script:
#!/bin/bash # Description: Check if the updated dependencies are used in the new webhook feature # Search for usage of @ballerine/workflow-core in TypeScript files echo "Searching for @ballerine/workflow-core usage:" rg '@ballerine/workflow-core' --glob '*.ts' --glob '*.tsx' echo "\nSearching for @ballerine/workflow-node-sdk usage:" rg '@ballerine/workflow-node-sdk' --glob '*.ts' --glob '*.tsx' # Check for any new imports in recently modified TypeScript files echo "\nChecking for new imports in recently modified files:" git diff --name-only HEAD~1 | grep -E '\.ts$|\.tsx$' | xargs rg '@ballerine/workflow-core|@ballerine/workflow-node-sdk'Length of output: 56069
sdks/workflow-node-sdk/CHANGELOG.md (1)
3-8: LGTM: Changelog updated correctly for version 0.6.45.The changelog has been updated appropriately to reflect the new version 0.6.45 and the dependency update for
@ballerine/workflow-core. This is good practice for maintaining clear version history.To ensure consistency, let's verify if the
package.jsonfile has been updated accordingly:✅ Verification successful
Verified:
package.jsoncorrectly updated for version 0.6.45.The
package.jsonfile accurately reflects the updated version0.6.45and the dependency@ballerine/workflow-corehas been updated to0.6.45. All changes are consistent with the changelog.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if package.json has been updated with the new version and dependency. # Test: Check package.json for version and dependency update jq '.version, .dependencies["@ballerine/workflow-core"]' sdks/workflow-node-sdk/package.jsonLength of output: 112
examples/headless-example/CHANGELOG.md (2)
3-7: LGTM. Verify compatibility with the updated dependency.The changelog entry for version 0.3.44 correctly documents the update of
@ballerine/workflow-browser-sdkto version 0.6.45. This patch update follows semantic versioning practices.To ensure compatibility, please run the following script to check for any breaking changes or deprecations in the updated dependency:
✅ Verification successful
✅ Compatibility Confirmed
No breaking changes or deprecations were found in the
@ballerine/workflow-browser-sdkchangelog for version 0.6.45. The patch update follows semantic versioning and appears safe to integrate.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for breaking changes or deprecations in @ballerine/workflow-browser-sdk # Test: Search for breaking changes or deprecations in the changelog of @ballerine/workflow-browser-sdk rg --type md -i "breaking change|deprecat" $(fd -t f CHANGELOG.md | grep workflow-browser-sdk)Length of output: 136
3-7: Consider updating @ballerine/common dependency.While version 0.3.44 updates
@ballerine/workflow-browser-sdk, it doesn't mention an update for@ballerine/common. The previous version (0.3.43) updated both dependencies. Please verify if@ballerine/commonalso needs to be updated in this version.Run the following script to check the latest version of
@ballerine/commonand compare it with the current version used in the project:apps/kyb-app/CHANGELOG.md (1)
1-8: Overall, the CHANGELOG.md update looks good.The new version entry (0.3.58) has been added correctly, maintaining consistency with previous entries. The update to the dependency is clearly stated. Consider adding brief descriptions of changes in future updates for improved clarity.
services/workflows-service/src/workflow/workflow.service.ts (2)
81-81: Imports are correctly added for new dependenciesThe imported modules
Customer,AwsSecretsManager, andInMemorySecretsManagerare necessary for the new functionality introduced in the code. The imports are correctly placed and follow the existing code conventions.Also applies to: 111-112
2050-2050: Verify the compatibility ofsecretsManagerincreateWorkflowIn the
createWorkflowinvocation, thesecretsManagerproperty is assigned an object with agetAllmethod returned bythis.getCustomerSecrets(secretsManager, customer). Ensure that this structure aligns with the expected interface ofsecretsManagerin thecreateWorkflowfunction and that it functions as intended during runtime.Run the following script to verify the structure of
secretsManager:✅ Verification successful
Verified
secretsManagercompatibility increateWorkflowThe
secretsManagerproperty increateWorkflowis correctly assigned an object with agetAllmethod as expected.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `secretsManager` parameter in `createWorkflow` has a `getAll` method. # Expected Result: Instances of `createWorkflow` should accept an object with a `getAll` function. # Find all occurrences of `createWorkflow` and check the structure of `secretsManager`. rg --type ts 'createWorkflow\(\{' -A 20 | grep 'secretsManager'Length of output: 63
Script:
#!/bin/bash # Description: Verify that the `secretsManager` parameter in all `createWorkflow` invocations includes a `getAll` method. # Expected Result: All instances of `createWorkflow` should pass an object with a `getAll` function for `secretsManager`. # Use ast-grep to find createWorkflow invocations and check the structure of secretsManager ast-grep --lang typescript --pattern $'createWorkflow({ $$$ secretsManager: { getAll: $_ } $$$ })' . --printLength of output: 335
Script:
#!/bin/bash # Description: Verify that the `secretsManager` parameter in all `createWorkflow` invocations includes a `getAll` method. # Expected Result: All instances of `createWorkflow` should pass an object with a `getAll` function for `secretsManager`. # Use ast-grep to find createWorkflow invocations and check the structure of secretsManager ast-grep --lang typescript --pattern 'createWorkflow({ $$$ secretsManager: { getAll: $_ } $$$ })' .Length of output: 14167
| @@ -1,7 +1,7 @@ | |||
| { | |||
| "name": "@ballerine/headless-example", | |||
| "private": true, | |||
| "version": "0.3.43", | |||
| "version": "0.3.44", | |||
There was a problem hiding this comment.
💡 Codebase verification
Action Required: Missing Changelog Entry for Dependency Update
The changelog does not include information for version 0.6.45 of @ballerine/workflow-browser-sdk. Please verify that this version includes the expected webhook functionality by checking the releases page or contacting the maintainer.
🔗 Analysis chain
LGTM: Version bump and dependency update look good.
The version bump from 0.3.43 to 0.3.44 is consistent with the update to the @ballerine/workflow-browser-sdk dependency (from 0.6.44 to 0.6.45). These changes align with the expected updates for introducing new functionality.
To ensure that the updated @ballerine/workflow-browser-sdk includes the expected webhook functionality, please run the following script:
This script will help locate and display relevant information about the changes in the updated dependency version.
Also applies to: 38-38
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the changelog or release notes for @ballerine/workflow-browser-sdk v0.6.45
# Test: Search for changelog or release notes
rg --type md -i "changelog|release notes" -g "!node_modules"
# If a changelog is found, check its content for version 0.6.45
changelog=$(rg --type md -i "changelog|release notes" -g "!node_modules" -l | head -n 1)
if [ -n "$changelog" ]; then
echo "Checking $changelog for version 0.6.45:"
rg -A 10 "0\.6\.45" "$changelog"
fi
# Check package.json for repository URL
repo_url=$(jq -r '.repository.url // .repository' package.json | sed 's/^git+//;s/\.git$//')
if [ -n "$repo_url" ]; then
echo "Repository URL: $repo_url"
echo "Please check the releases page for version 0.6.45 of @ballerine/workflow-browser-sdk"
fi
Length of output: 1432
| private getCustomerSecrets( | ||
| secretsManager: AwsSecretsManager | InMemorySecretsManager, | ||
| { authenticationConfiguration }: Customer, | ||
| ) { | ||
| return async () => { | ||
| const secrets = await secretsManager.getAll(); | ||
| const webhookSharedSecret = authenticationConfiguration?.webhookSharedSecret; | ||
|
|
||
| return { | ||
| ...(webhookSharedSecret ? { webhookSharedSecret } : {}), | ||
| ...secrets, | ||
| } as Record<string, string>; | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure proper error handling in getCustomerSecrets
The getCustomerSecrets method retrieves secrets from the secretsManager and combines them with the webhookSharedSecret. Consider adding error handling to manage potential exceptions when fetching secrets, such as network issues or permission errors. This will enhance the robustness of the method.
Apply this diff to include try-catch blocks for error handling:
return async () => {
- const secrets = await secretsManager.getAll();
- const webhookSharedSecret = authenticationConfiguration?.webhookSharedSecret;
+ try {
+ const secrets = await secretsManager.getAll();
+ const webhookSharedSecret = authenticationConfiguration?.webhookSharedSecret;
- return {
- ...(webhookSharedSecret ? { webhookSharedSecret } : {}),
- ...secrets,
- } as Record<string, string>;
+ return {
+ ...(webhookSharedSecret ? { webhookSharedSecret } : {}),
+ ...secrets,
+ } as Record<string, string>;
+ } catch (error) {
+ this.logger.error('Failed to retrieve customer secrets', { error });
+ throw new InternalServerErrorException('Unable to retrieve customer secrets.');
+ }
};Committable suggestion was skipped due to low confidence.
| const customer = await this.customerService.getByProjectId(projectIds![0]!, { | ||
| select: { | ||
| id: true, | ||
| name: true, | ||
| displayName: true, | ||
| logoImageUri: true, | ||
| faviconImageUri: true, | ||
| country: true, | ||
| language: true, | ||
| websiteUrl: true, | ||
| projects: true, | ||
| subscriptions: true, | ||
| config: true, | ||
| authenticationConfiguration: true, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Ensure projectIds is not null or empty before accessing
The code accesses projectIds![0]! assuming that projectIds is not null and contains at least one element. If projectIds is null or empty, this will result in a runtime error. Consider adding a check to ensure projectIds is not null and has at least one element before accessing projectIds[0].
Apply this diff to add a validation check:
const customerProjectId = projectIds && projectIds[0];
+ if (!customerProjectId) {
+ throw new Error('No project ID found. Please ensure that projectIds is not null or empty.');
+ }
const customer = await this.customerService.getByProjectId(customerProjectId, {
select: {
id: true,
name: true,
displayName: true,
logoImageUri: true,
faviconImageUri: true,
country: true,
language: true,
websiteUrl: true,
projects: true,
subscriptions: true,
config: true,
authenticationConfiguration: true,
},
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const customer = await this.customerService.getByProjectId(projectIds![0]!, { | |
| select: { | |
| id: true, | |
| name: true, | |
| displayName: true, | |
| logoImageUri: true, | |
| faviconImageUri: true, | |
| country: true, | |
| language: true, | |
| websiteUrl: true, | |
| projects: true, | |
| subscriptions: true, | |
| config: true, | |
| authenticationConfiguration: true, | |
| }, | |
| }); | |
| const customerProjectId = projectIds && projectIds[0]; | |
| if (!customerProjectId) { | |
| throw new Error('No project ID found. Please ensure that projectIds is not null or empty.'); | |
| } | |
| const customer = await this.customerService.getByProjectId(customerProjectId, { | |
| select: { | |
| id: true, | |
| name: true, | |
| displayName: true, | |
| logoImageUri: true, | |
| faviconImageUri: true, | |
| country: true, | |
| language: true, | |
| websiteUrl: true, | |
| projects: true, | |
| subscriptions: true, | |
| config: true, | |
| authenticationConfiguration: true, | |
| }, | |
| }); |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/common/package.json (1)
79-80: Consider security and licensing implications of adding crypto-js.The addition of
crypto-jsand its type definitions enhances the package's capabilities but also introduces some considerations:
Security: Ensure that the cryptographic functions are used correctly and securely. Consider having a security review if this introduces significant new cryptographic operations.
Licensing: Verify that the Apache-2.0 license of this package is compatible with the MIT license of
crypto-js.Documentation: Update the package's documentation to reflect the new cryptographic capabilities.
Testing: Ensure that any new functionality using
crypto-jsis adequately tested.Also applies to: 85-85
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
- packages/common/package.json (1 hunks)
🔇 Additional comments (2)
packages/common/package.json (2)
79-80: LGTM: Addition of crypto-js type definitions.The addition of
@types/crypto-jsto devDependencies is appropriate. It provides TypeScript type definitions for the crypto-js library, which will enhance development experience and type safety when using crypto-js in the project.
85-85: Approved, but please provide more context on crypto-js usage.The addition of
crypto-js(version ^4.2.0) to the dependencies is noted. This library provides cryptographic functions, which aligns with the PR's objective of implementing webhook functionality (possibly for signing or verifying webhooks).However, to ensure proper usage and security:
- Could you please elaborate on the specific use cases for
crypto-jsin this package?- Have you considered any potential security implications of using this library?
- Is there a reason for choosing
crypto-jsover Node.js's built-incryptomodule?To help verify the intended usage, please run the following script:
✅ Verification successful
crypto-js Usage Verified
The
crypto-jslibrary is imported in./packages/common/src/utils/sign/sign.ts, confirming its use for cryptographic operations related to webhook functionality.Please ensure that its implementation follows security best practices.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for crypto-js imports and usages echo "Searching for crypto-js imports and usages:" rg --type typescript --type javascript 'import.*crypto-js' ./packages/common rg --type typescript --type javascript 'require.*crypto-js' ./packages/common rg --type typescript --type javascript 'crypto-js' ./packages/commonLength of output: 429
Script:
#!/bin/bash # Search for crypto-js imports and usages with corrected type flags echo "Searching for crypto-js imports and usages:" rg --type ts --type js 'import.*crypto-js' ./packages/common rg --type ts --type js 'require.*crypto-js' ./packages/common rg --type ts --type js 'crypto-js' ./packages/commonLength of output: 426
feat: added webhook implementing webhook shared
Summary by CodeRabbit
New Features
Bug Fixes
Documentation