-
Notifications
You must be signed in to change notification settings - Fork 76
Validate CoinPay CLI payment arguments #629
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,101 @@ | ||
| import { contractTestTarget } from '@profullstack/sh1pt-core/testing'; | ||
| import { contractTestTarget, fakeShipContext } from '@profullstack/sh1pt-core/testing'; | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| const { execMock } = vi.hoisted(() => ({ | ||
| execMock: vi.fn(), | ||
| })); | ||
|
|
||
| vi.mock('@profullstack/sh1pt-core', async () => ({ | ||
| ...await vi.importActual<typeof import('@profullstack/sh1pt-core')>('@profullstack/sh1pt-core'), | ||
| exec: execMock, | ||
| })); | ||
|
|
||
| import target from './index.js'; | ||
|
|
||
| contractTestTarget(target, { | ||
| sampleConfig: { command: 'create', args: { amount: 100, blockchain: 'BTC' }, businessId: 'biz_test' }, | ||
| requiredSecrets: ['COINPAY_API_KEY'], | ||
| }); | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| execMock.mockResolvedValue({ exitCode: 0, stdout: '{"ok":true}', stderr: '' }); | ||
| }); | ||
|
|
||
| describe('payment-coinpay target adapter', () => { | ||
| it('creates payments with validated amount, businessId, and blockchain', async () => { | ||
| const ctx = fakeShipContext({ dryRun: false }); | ||
|
|
||
| await target.ship(ctx as any, { | ||
| command: 'create', | ||
| businessId: 'biz_test', | ||
| args: { amount: 25.5, blockchain: 'sol' }, | ||
| description: 'Test payment', | ||
| }); | ||
|
|
||
| expect(execMock).toHaveBeenCalledWith('coinpay', [ | ||
| 'payment', | ||
| 'create', | ||
| '--business-id', | ||
| 'biz_test', | ||
| '--amount', | ||
| '25.5', | ||
| '--blockchain', | ||
| 'SOL', | ||
| '--description', | ||
| 'Test payment', | ||
| ], { | ||
| log: ctx.log, | ||
| throwOnNonZero: true, | ||
| }); | ||
| }); | ||
|
|
||
| it('rejects invalid create amounts before invoking the CLI', async () => { | ||
| await expect(target.ship(fakeShipContext({ dryRun: false }) as any, { | ||
| command: 'create', | ||
| args: { amount: 0, blockchain: 'BTC' }, | ||
| })).rejects.toThrow('amount must be a positive number'); | ||
|
|
||
| expect(execMock).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('rejects blank business IDs before invoking the CLI', async () => { | ||
| await expect(target.ship(fakeShipContext({ dryRun: false }) as any, { | ||
| command: 'create', | ||
| businessId: ' ', | ||
| args: { amount: 10, blockchain: 'BTC' }, | ||
| })).rejects.toThrow('businessId required'); | ||
|
|
||
| expect(execMock).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('requires paymentId for get commands', async () => { | ||
| await expect(target.ship(fakeShipContext({ dryRun: false }) as any, { | ||
| command: 'get', | ||
| args: { paymentId: ' ' }, | ||
| })).rejects.toThrow('paymentId required'); | ||
|
|
||
| expect(execMock).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('normalizes rate asset codes before invoking the CLI', async () => { | ||
| const ctx = fakeShipContext({ dryRun: false }); | ||
|
|
||
| await target.ship(ctx as any, { | ||
| command: 'rates', | ||
| args: { coin: 'sol', fiat: 'usd' }, | ||
| }); | ||
|
|
||
| expect(execMock).toHaveBeenCalledWith('coinpay', ['rates', 'get', 'SOL', '--fiat', 'USD'], { | ||
| log: ctx.log, | ||
| }); | ||
| }); | ||
|
|
||
| it('rejects malformed rate asset codes before invoking the CLI', async () => { | ||
| await expect(target.ship(fakeShipContext({ dryRun: false }) as any, { | ||
| command: 'rates', | ||
| args: { coin: 'sol-mainnet', fiat: 'USD' }, | ||
| })).rejects.toThrow('coin must be an uppercase asset code'); | ||
|
|
||
| expect(execMock).not.toHaveBeenCalled(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,30 @@ interface Config { | |
| description?: string; | ||
| } | ||
|
|
||
| function requireText(value: unknown, name: string): string { | ||
| if (typeof value !== 'string' || !value.trim()) throw new Error(`${name} required`); | ||
| return value.trim(); | ||
| } | ||
|
|
||
| function optionalText(value: unknown, name: string): string | undefined { | ||
| if (value === undefined) return undefined; | ||
| return requireText(value, name); | ||
| } | ||
|
|
||
| function requirePositiveAmount(value: unknown): number { | ||
| if (typeof value !== 'number' || !Number.isFinite(value) || value <= 0) { | ||
| throw new Error('amount must be a positive number'); | ||
| } | ||
| return value; | ||
| } | ||
|
|
||
| function requireAssetCode(value: unknown, name: string, fallback?: string): string { | ||
| const raw = value === undefined ? fallback : value; | ||
| const code = requireText(raw, name).toUpperCase(); | ||
| if (!/^[A-Z0-9_]{2,16}$/.test(code)) throw new Error(`${name} must be an uppercase asset code`); | ||
| return code; | ||
| } | ||
|
|
||
| export default defineTarget<Config>({ | ||
| id: 'payment-coinpay', | ||
| kind: 'payment', | ||
|
|
@@ -55,34 +79,34 @@ export default defineTarget<Config>({ | |
| switch (cmd) { | ||
| case 'create': { | ||
| const args = ['payment', 'create']; | ||
| const bizId = config.businessId ?? (config.args?.businessId as string); | ||
| const bizId = optionalText(config.businessId ?? config.args?.businessId, 'businessId'); | ||
| if (bizId) args.push('--business-id', bizId); | ||
| if (config.args?.amount) args.push('--amount', String(config.args.amount)); | ||
| if (config.args?.blockchain) args.push('--blockchain', String(config.args.blockchain)); | ||
| const amount = requirePositiveAmount(config.args?.amount); | ||
| args.push('--amount', String(amount)); | ||
| if (config.args?.blockchain) args.push('--blockchain', requireAssetCode(config.args.blockchain, 'blockchain')); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| if (config.description) args.push('--description', config.description); | ||
|
|
||
| const { stdout } = await exec('coinpay', args, { log: ctx.log, throwOnNonZero: true }); | ||
| return { id: `cp_${Date.now()}`, meta: { raw: stdout.trim() } }; | ||
| } | ||
|
|
||
| case 'get': { | ||
| const paymentId = config.args?.paymentId as string; | ||
| if (!paymentId) throw new Error('paymentId required for get command'); | ||
| const paymentId = requireText(config.args?.paymentId, 'paymentId'); | ||
| const { stdout } = await exec('coinpay', ['payment', 'get', paymentId], { log: ctx.log }); | ||
| return { id: paymentId, meta: { raw: stdout.trim() } }; | ||
| } | ||
|
|
||
| case 'list': { | ||
| const args = ['payment', 'list']; | ||
| const bizId = config.businessId ?? (config.args?.businessId as string); | ||
| const bizId = optionalText(config.businessId ?? config.args?.businessId, 'businessId'); | ||
| if (bizId) args.push('--business-id', bizId); | ||
| const { stdout } = await exec('coinpay', args, { log: ctx.log }); | ||
| return { id: `list-${Date.now()}`, meta: { raw: stdout.trim() } }; | ||
| } | ||
|
|
||
| case 'rates': { | ||
| const coin = (config.args?.coin as string) ?? 'BTC'; | ||
| const fiat = (config.args?.fiat as string) ?? 'USD'; | ||
| const coin = requireAssetCode(config.args?.coin, 'coin', 'BTC'); | ||
| const fiat = requireAssetCode(config.args?.fiat, 'fiat', 'USD'); | ||
| const { stdout } = await exec('coinpay', ['rates', 'get', coin, '--fiat', fiat], { log: ctx.log }); | ||
| return { id: `${coin}-${fiat}`, meta: { raw: stdout.trim() } }; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
requireAssetCodeonly fires whenvalue === undefined, which means explicitly passingnull(a validunknownvalue inRecord<string, unknown>) bypasses the default and falls through torequireText(null, name), which throws"coin required"instead of using the fallback. The more conventional JavaScript idiom for "treat null and undefined the same" is nullish coalescing, which would correctly apply the fallback for both.