#321 feat(cli): add --dry-run to 'alerts test' to preview payload without sending FIXED - #582
Conversation
…load without sending FIXED
|
@veloura-dev Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds ChangesAlert dry-run preview
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| - | - | Generic High Entropy Secret | ded54f4 | tests/commands/guard-cli-export-import.test.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/commands/alerts.test.ts`:
- Around line 811-816: Strengthen the assertion around signatureCall in the
alert test to validate the complete X-Sorokeep-Signature header, not only its
prefix. Derive the expected HMAC using the test secret and printed payload, then
compare the logged header exactly against the corresponding sha256 signature.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 94ff9e7e-7112-48a5-8f90-8215f974d3d2
📒 Files selected for processing (2)
src/commands/alerts.tstests/commands/alerts.test.ts
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: CI Pipeline / 0_build-and-test (22.x).txt: #321 feat(cli): add --dry-run to 'alerts test' to preview payload without sending FIXED
Conclusion: failure
##[group]Run npm audit --audit-level=high
�[36;1mnpm audit --audit-level=high�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
# npm audit report
`@hono/node-server` <2.0.5
Severity: moderate
Node.js Adapter for Hono: Path traversal in `serve-static` on Windows via encoded backslash (`%5C`) - https://github.com/advisories/GHSA-frvp-7c67-39w9
fix available via `npm audit fix`
node_modules/@hono/node-server
`@modelcontextprotocol/sdk` 1.25.0 - 1.29.0
Depends on vulnerable versions of `@hono/node-server`
node_modules/@modelcontextprotocol/sdk
axios 1.0.0 - 1.17.0
Severity: high
Axios: Excessive recursion in formDataToJSON can cause denial of service - https://github.com/advisories/GHSA-42h9-826w-cgv3
Axios: Prototype pollution auth subfields can inject Basic auth - https://github.com/advisories/GHSA-xj6q-8x83-jv6g
Axios: Deep formToJSON Key Recursion Can Cause Denial of Service - https://github.com/advisories/GHSA-pmv8-rq9r-6j72
Axios: Fetch adapter `ReadableStream` uploads bypass `maxBodyLength` - https://github.com/advisories/GHSA-jqh4-m9w3-8hp9
Axios: Prototype pollution gadgets can alter axios request construction - https://github.com/advisories/GHSA-mmx7-hfxf-jppx
Axios: NO_PROXY bypass for 0.0.0.0 local addresses in axios - https://github.com/advisories/GHSA-f4gw-2p7v-4548
Axios Node HTTP adapter can use an inherited proxy after interceptor config cloning - https://github.com/advisories/GHSA-gcfj-64vw-6mp9
Axios form serializer maxDepth bypass via {} metatoken - https://github.com/advisories/GHSA-hcpx-6fm6-wx23
Axios: Nested axios option objects can consume polluted prototype values - https://github.com/advisories/GHSA-7q8q-rj6j-mhjq
Axios: HTTP/2 streamed uploads bypass `maxBodyLength` - https://github.com/advisories/GHSA-mwf2-3pr3-8698
fix available via `npm audit fix`
node_modules/axios
`@stellar/stellar-sdk` 15.0.1 - 16.0.1
Depends on vulnerable versions of axios
node_modules/@stellar/stellar-sdk
brace-expansion <=5.0.7
...
GitHub Actions: CI Pipeline / build-and-test (22.x): #321 feat(cli): add --dry-run to 'alerts test' to preview payload without sending FIXED
Conclusion: failure
##[group]Run npm audit --audit-level=high
�[36;1mnpm audit --audit-level=high�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
# npm audit report
`@hono/node-server` <2.0.5
Severity: moderate
Node.js Adapter for Hono: Path traversal in `serve-static` on Windows via encoded backslash (`%5C`) - https://github.com/advisories/GHSA-frvp-7c67-39w9
fix available via `npm audit fix`
node_modules/@hono/node-server
`@modelcontextprotocol/sdk` 1.25.0 - 1.29.0
Depends on vulnerable versions of `@hono/node-server`
node_modules/@modelcontextprotocol/sdk
axios 1.0.0 - 1.17.0
Severity: high
Axios: Excessive recursion in formDataToJSON can cause denial of service - https://github.com/advisories/GHSA-42h9-826w-cgv3
Axios: Prototype pollution auth subfields can inject Basic auth - https://github.com/advisories/GHSA-xj6q-8x83-jv6g
Axios: Deep formToJSON Key Recursion Can Cause Denial of Service - https://github.com/advisories/GHSA-pmv8-rq9r-6j72
Axios: Fetch adapter `ReadableStream` uploads bypass `maxBodyLength` - https://github.com/advisories/GHSA-jqh4-m9w3-8hp9
Axios: Prototype pollution gadgets can alter axios request construction - https://github.com/advisories/GHSA-mmx7-hfxf-jppx
Axios: NO_PROXY bypass for 0.0.0.0 local addresses in axios - https://github.com/advisories/GHSA-f4gw-2p7v-4548
Axios Node HTTP adapter can use an inherited proxy after interceptor config cloning - https://github.com/advisories/GHSA-gcfj-64vw-6mp9
Axios form serializer maxDepth bypass via {} metatoken - https://github.com/advisories/GHSA-hcpx-6fm6-wx23
Axios: Nested axios option objects can consume polluted prototype values - https://github.com/advisories/GHSA-7q8q-rj6j-mhjq
Axios: HTTP/2 streamed uploads bypass `maxBodyLength` - https://github.com/advisories/GHSA-mwf2-3pr3-8698
fix available via `npm audit fix`
node_modules/axios
`@stellar/stellar-sdk` 15.0.1 - 16.0.1
Depends on vulnerable versions of axios
node_modules/@stellar/stellar-sdk
brace-expansion <=5.0.7
...
🔇 Additional comments (2)
src/commands/alerts.ts (1)
17-17: LGTM!Also applies to: 210-210, 238-259
tests/commands/alerts.test.ts (1)
763-787: LGTM!
| // Should print X-Sorokeep-Signature: sha256=<hmac> | ||
| const signatureCall = consoleLogSpy.mock.calls.find((args) => | ||
| typeof args[0] === "string" && args[0].startsWith("X-Sorokeep-Signature:") | ||
| ); | ||
| expect(signatureCall).toBeTruthy(); | ||
| expect(signatureCall![0]).toContain("X-Sorokeep-Signature: sha256="); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the signature value, not just its prefix.
This passes for any sha256= value, including a signature generated with the wrong secret or body. Compute the expected HMAC over the printed payload and compare the complete header.
Proposed test assertion
+ const { createHmac } = await import("node:crypto");
+ const bodyCall = consoleLogSpy.mock.calls.find(([value]) => {
+ try {
+ return JSON.parse(value).type === "threshold_crossed";
+ } catch {
+ return false;
+ }
+ });
+
expect(signatureCall).toBeTruthy();
- expect(signatureCall![0]).toContain("X-Sorokeep-Signature: sha256=");
+ const expected = createHmac("sha256", "dry-run-secret")
+ .update(bodyCall![0])
+ .digest("hex");
+ expect(signatureCall![0]).toBe(`X-Sorokeep-Signature: sha256=${expected}`);📝 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.
| // Should print X-Sorokeep-Signature: sha256=<hmac> | |
| const signatureCall = consoleLogSpy.mock.calls.find((args) => | |
| typeof args[0] === "string" && args[0].startsWith("X-Sorokeep-Signature:") | |
| ); | |
| expect(signatureCall).toBeTruthy(); | |
| expect(signatureCall![0]).toContain("X-Sorokeep-Signature: sha256="); | |
| const { createHmac } = await import("node:crypto"); | |
| const bodyCall = consoleLogSpy.mock.calls.find(([value]) => { | |
| try { | |
| return JSON.parse(value).type === "threshold_crossed"; | |
| } catch { | |
| return false; | |
| } | |
| }); | |
| // Should print X-Sorokeep-Signature: sha256=<hmac> | |
| const signatureCall = consoleLogSpy.mock.calls.find((args) => | |
| typeof args[0] === "string" && args[0].startsWith("X-Sorokeep-Signature:") | |
| ); | |
| expect(signatureCall).toBeTruthy(); | |
| const expected = createHmac("sha256", "dry-run-secret") | |
| .update(bodyCall![0]) | |
| .digest("hex"); | |
| expect(signatureCall![0]).toBe(`X-Sorokeep-Signature: sha256=${expected}`); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/commands/alerts.test.ts` around lines 811 - 816, Strengthen the
assertion around signatureCall in the alert test to validate the complete
X-Sorokeep-Signature header, not only its prefix. Derive the expected HMAC using
the test secret and printed payload, then compare the logged header exactly
against the corresponding sha256 signature.
|
Merged into |
3 participants
What does this PR do?
This PR adds a
--dry-runflag to thesorokeep alerts testcommand insrc/commands/alerts.ts. When this flag is provided, the command constructs the testAlertEventand prints the exact JSON payload (as well as the computedX-Sorokeep-Signatureheader for signed webhooks) directly to the console instead of dispatching the alert viadeliverSingleAlert.Closes #321
Why?
When configuring a new webhook or notification receiver, developers need a way to inspect the exact payload structure and HTTP headers (specifically for payload validation using webhook signing secrets) that
sorokeeptransmits. The--dry-runflag enables offline validation and debugging without executing actual HTTP requests or triggering destination alerting systems.Does this touch secret-key handling or transaction submission?
Yes — see notes above
No
Checklist
Tests pass (
npm test)Type check passes (
npx tsc --noEmit)Lint passes (
npm run lint)Tests cover the new functionality (TDD preferred — see CONTRIBUTING.md)
No unnecessary dependencies added
Commit messages follow conventional format
No
console.login core logicADR added if this is a significant design decision (see docs/adr)
E2E sandbox tested, if this touches RPC or daemon behavior (see docs/e2e-sandbox.md)