Repository navigation
feat: add guard command tests for --auto-extend policy registration - #307
Conversation
- Add unit tests covering all validation paths (invalid TTL, threshold >= target, missing keypair-env for --auto-extend, dry-run edge cases) - Add integration tests verifying acceptance criteria: - --auto-extend registers extension policy in SQLite - Only public key (not secret) is stored in DB; keypair_source stores env var reference
|
@JClark011 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
Walkthrough
ChangesGuard Command Test Refactor and Integration Coverage
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/guard.test.ts`:
- Around line 202-204: The test cleanup for STELLAR_TEST_KEY is currently
happening inline and can be skipped if parseAsync or an assertion throws,
causing leakage into later tests. Move the env reset into the existing afterEach
cleanup in guard.test.ts alongside vi.restoreAllMocks(), and use the relevant
test blocks around parseAsync/assertions to rely on that shared teardown. Keep
the cleanup centralized so every case in this suite gets the same env reset,
including the affected guard test sections.
- Around line 30-35: The resolveSecretKey mock in the guard tests is too
permissive because every env:* source returns the same secret, hiding bad
env-name handling. Update the mock in guard.test.ts so it parses the requested
env variable name from the env:<name> source and returns the corresponding value
from process.env, while keeping the vault:* branch as-is. Use the
resolveSecretKey mock and the env:/vault: source handling as the unique points
to locate the change.
- Around line 10-11: The test constants in guard.test.ts include a real-looking
Stellar secret key that will trigger secret scanning. Replace the hardcoded
VALID_TEST_SECRET and VALID_TEST_PUBKEY values by generating a deterministic
test keypair at runtime inside the relevant test setup, and update any tests
that reference those symbols to use the generated values instead of committed
secrets.
🪄 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: 671c2cbe-4ff8-4225-955a-10362de021c3
📒 Files selected for processing (1)
tests/commands/guard.test.ts
📜 Review details
🧰 Additional context used
🪛 Betterleaks (1.6.0)
tests/commands/guard.test.ts
[high] 10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
| const VALID_TEST_SECRET = "SCG2IACKCYEUMINFHVGAOB3UFDVSVRACCZJH4K3R6WVC2OTRDQPK2GWG"; | ||
| const VALID_TEST_PUBKEY = "GA4YORXJVEPWAYDHC3AAFGUJRWCCO3GOP3T226ZFKWSLUCAYS7NKRLUU"; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify no Stellar secret keys remain hard-coded.
rg -nP '\bS[A-Z2-7]{55}\b' --glob '!**/node_modules/**' .Repository: AbdulmalikAlayande/sorokeep
Length of output: 821
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant section of the test file.
sed -n '1,280p' tests/commands/guard.test.ts
echo
echo '---'
echo '# Related env/setup lines in the same file'
rg -n 'STELLAR_TEST_KEY|resolveSecretKey|afterEach|beforeEach|delete process\.env|process\.env' tests/commands/guard.test.tsRepository: AbdulmalikAlayande/sorokeep
Length of output: 12370
Avoid committing a real Stellar secret key. This valid-looking secret will trigger secret scanning; generate the test keypair at runtime instead.
🧰 Tools
🪛 Betterleaks (1.6.0)
[high] 10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 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/guard.test.ts` around lines 10 - 11, The test constants in
guard.test.ts include a real-looking Stellar secret key that will trigger secret
scanning. Replace the hardcoded VALID_TEST_SECRET and VALID_TEST_PUBKEY values
by generating a deterministic test keypair at runtime inside the relevant test
setup, and update any tests that reference those symbols to use the generated
values instead of committed secrets.
Source: Linters/SAST tools
| resolveSecretKey: vi.fn(async (source: string) => { | ||
| // For tests: if source looks like env: or vault:, return a fake valid key | ||
| // Otherwise assume it's a direct secret key and return it | ||
| if (source.startsWith("env:") || source.startsWith("vault:")) { | ||
| return "SA7QYNF7SOWQ3GLR" + "2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJVSGZ"; | ||
| return VALID_TEST_SECRET; | ||
| } | ||
| return source; | ||
| }), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the env resolver mock read the requested env var.
Right now any env:* source returns VALID_TEST_SECRET, so the integration tests would pass even if the env var is unset or the source name is mishandled. Parse env:<name> from process.env to exercise the CLI/env contract.
Suggested fix
resolveSecretKey: vi.fn(async (source: string) => {
- if (source.startsWith("env:") || source.startsWith("vault:")) {
+ if (source.startsWith("env:")) {
+ return process.env[source.slice("env:".length)] ?? null;
+ }
+ if (source.startsWith("vault:")) {
return VALID_TEST_SECRET;
}
return source;
}),📝 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.
| resolveSecretKey: vi.fn(async (source: string) => { | |
| // For tests: if source looks like env: or vault:, return a fake valid key | |
| // Otherwise assume it's a direct secret key and return it | |
| if (source.startsWith("env:") || source.startsWith("vault:")) { | |
| return "SA7QYNF7SOWQ3GLR" + "2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJVSGZ"; | |
| return VALID_TEST_SECRET; | |
| } | |
| return source; | |
| }), | |
| resolveSecretKey: vi.fn(async (source: string) => { | |
| if (source.startsWith("env:")) { | |
| return process.env[source.slice("env:".length)] ?? null; | |
| } | |
| if (source.startsWith("vault:")) { | |
| return VALID_TEST_SECRET; | |
| } | |
| return source; | |
| }), |
🤖 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/guard.test.ts` around lines 30 - 35, The resolveSecretKey mock
in the guard tests is too permissive because every env:* source returns the same
secret, hiding bad env-name handling. Update the mock in guard.test.ts so it
parses the requested env variable name from the env:<name> source and returns
the corresponding value from process.env, while keeping the vault:* branch
as-is. Use the resolveSecretKey mock and the env:/vault: source handling as the
unique points to locate the change.
| afterEach(() => { | ||
| vi.restoreAllMocks(); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Move env cleanup into afterEach.
If parseAsync or an assertion throws before the inline delete, STELLAR_TEST_KEY leaks into later tests.
Suggested fix
afterEach(() => {
+ delete process.env.STELLAR_TEST_KEY;
vi.restoreAllMocks();
});
...
- delete process.env.STELLAR_TEST_KEY;
});
...
- delete process.env.STELLAR_TEST_KEY;
});Also applies to: 207-227, 231-255
🤖 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/guard.test.ts` around lines 202 - 204, The test cleanup for
STELLAR_TEST_KEY is currently happening inline and can be skipped if parseAsync
or an assertion throws, causing leakage into later tests. Move the env reset
into the existing afterEach cleanup in guard.test.ts alongside
vi.restoreAllMocks(), and use the relevant test blocks around
parseAsync/assertions to rely on that shared teardown. Keep the cleanup
centralized so every case in this suite gets the same env reset, including the
affected guard test sections.
…307) - Add unit tests covering all validation paths (invalid TTL, threshold >= target, missing keypair-env for --auto-extend, dry-run edge cases) - Add integration tests verifying acceptance criteria: - --auto-extend registers extension policy in SQLite - Only public key (not secret) is stored in DB; keypair_source stores env var reference
…307) - Add unit tests covering all validation paths (invalid TTL, threshold >= target, missing keypair-env for --auto-extend, dry-run edge cases) - Add integration tests verifying acceptance criteria: - --auto-extend registers extension policy in SQLite - Only public key (not secret) is stored in DB; keypair_source stores env var reference
2 participants
feat: implement guard command with auto-extension policy support
Summary
Adds the sorokeep guard CLI command, which allows developers to configure and
enable automatic TTL extension policies for watched Soroban contracts.
Changes
--target-ttl, --threshold, --keypair, --keypair-env, --keypair-vault, --auto-extend,
--dry-run, --disable
criteria
Behaviour
automatically when they drop below the configured threshold
Security
Secret keys are never stored in the database. When --auto-extend is configured, only the
derived public key and the environment variable name (e.g. env:STELLAR_SECRET_KEY) are
persisted. The daemon resolves the actual secret from the environment at runtime.
Validation
for daemon operation)
Tests
716 passed | 1 skipped — 0 failures
Covers: contract-not-found, invalid TTL inputs, threshold ≥ target, --disable, policy
display, dry-run with/without keypair, one-time extension, and two integration tests that
verify the acceptance criteria against a real in-memory SQLite database.
Close #118