parseCSV() silently drops rows with unparseable amounts and lets parseFloat silently truncate malformed numeric strings

[prodbycorne]

Overview

parseCSV() (the CSV-to-recipients parser backing the airdrop recipients file-upload path) silently drops any row whose amount field doesn't parse to a valid number, and uses parseFloat, which itself silently truncates malformed numeric strings rather than rejecting them — with no error, no warning, and no discrepancy report telling the uploader that some rows were excluded or misinterpreted.

async function parseCSV(buffer) {
  const results = [];
  let rowCount = 0;
  ...
  await pipeline(Readable.from(chunks), csv(), async (rows) => {
    for await (const data of rows) {
      rowCount += 1;
      if (rowCount > config.airdrops.maxRecipients) {
        throw new AppError('VALIDATION_ERROR', 'recipients cannot exceed 10,000', 400);
      }

      const address = data.address || data.Address || data.ADDRESS;
      const amount = parseFloat(data.amount || data.Amount || data.AMOUNT);
      if (address && !Number.isNaN(amount)) {
        results.push({ address, amount });
      }
    }
  });

  return results;
}

Two distinct, confirmed problems in this function, both silent:

1. Rows with an unparseable amount are dropped, not rejected. if (address && !Number.isNaN(amount)) is the only gate on whether a parsed row makes it into results — a row with a missing, empty, or entirely non-numeric amount field (e.g. a blank cell, a stray text note in that column, a header-row mismatch) simply doesn't get pushed into results, with rowCount still incremented (so it does count toward the 10,000-row cap) but the row itself vanishing from the output with zero indication to the caller that anything was excluded. The function's return value — a plain array — carries no metadata about how many input rows were seen vs. how many were actually parsed into results; a caller cannot distinguish "this CSV genuinely had exactly 47 recipient rows" from "this CSV had 50 rows but 3 were silently dropped for having unparseable amounts."
2. parseFloat silently truncates malformed numeric strings instead of rejecting them. parseFloat parses as much of a leading numeric substring as it can and silently ignores everything after — parseFloat("1,000") returns 1 (stops at the comma), parseFloat("100USD") returns 100, parseFloat("50 units") returns 50. A CSV exported from a spreadsheet using thousands-separator formatting (an extremely common, realistic real-world CSV shape for a "recipient amounts" file) would have every comma-formatted value silently truncated down to just its leading digit group — "1,000,000" becomes 1, "25,000" becomes 25 — with no error, no rejected row, nothing distinguishing a correctly-parsed small amount from a catastrophically mis-parsed large one. This is a financial-disbursement input path (parseCSV's output ultimately becomes the recipient amounts for a token airdrop), and it accepts and silently mangles malformed numeric input rather than validating it strictly.

This is not merely a false sense of security from the existing total-amount-vs-sum validation (open issue #72, total !== body.total_amount for the JSON-body create path) — the CSV-upload path (POST /airdrops/:id/recipients with a file) adds recipients to an existing airdrop's already-fixed total_amount and does not perform any sum-against-total_amount cross-check at all for CSV-derived recipients (that check only exists in airdropCreateBodySchema's superRefine, which only runs for the JSON-body creation path, not the recipients-upload path) — so a silently truncated or silently dropped row in a CSV upload has no independent safety net catching it at all on this specific path.

Requirements

• Make parseCSV() reject (fail the whole upload with a clear, row-numbered validation error) rather than silently drop any row with a missing or unparseable amount — consistent with how every other validation failure in this same handler (invalid address, duplicate address, non-positive amount) is already surfaced as an explicit, row-numbered AppError, not silently dropped.
• Replace parseFloat with strict numeric parsing that rejects (rather than truncates) any string containing trailing non-numeric content after the numeric portion — e.g. validate with a regex (/^-?\d+(\.\d+)?$/) or Number(str) (which returns NaN for "1,000"/"100USD" rather than silently truncating, unlike parseFloat) before accepting a row's amount.
• If some degree of leniency is genuinely desired (e.g. stripping thousands-separator commas before parsing, as a deliberate, explicit accommodation for spreadsheet-exported CSVs), do so explicitly and only for that specific, documented format — not as an incidental side effect of using a lenient parsing function that also happens to accept arbitrary trailing garbage.
• Report, at minimum in an error case, which row number(s) failed and why, matching the row-numbered error style already used elsewhere in the same file (e.g. `recipient ${i}: invalid Stellar address`).

Acceptance Criteria

• A CSV row with a missing, empty, or non-numeric amount field causes the entire upload to be rejected with a clear, row-numbered error — not silently excluded from the parsed result.
• A CSV row with a comma-formatted amount (e.g. "1,000") is either explicitly, deliberately accepted as 1000 (if comma-stripping is intentionally implemented) or explicitly rejected with a clear error — it must not silently become 1.
• A CSV row with trailing non-numeric garbage in the amount field (e.g. "100USD", "50 units") is rejected with a clear error, not silently truncated to 100/50.
• A test exercises all three malformed-amount scenarios above (missing, comma-formatted, trailing-garbage) and asserts the upload is rejected (or, for the comma case, correctly and deliberately handled) rather than silently producing a wrong or incomplete recipient list.

Additional Notes

More precise references

• src/routes/airdrops.js:139-141 (parseCSV's row-processing body): confirmed the exact code — const amount = parseFloat(data.amount || data.Amount || data.AMOUNT); if (address && !Number.isNaN(amount)) { results.push({ address, amount }); } — no else branch, no accumulation of skipped-row information, no error thrown for a row failing this check.
• src/routes/airdrops.js:135-137: confirmed rowCount (used only for the 10,000-row cap check) is incremented for every row read from the CSV stream regardless of whether that row ends up in results — confirming the cap-check and the actual-output-count are tracked independently, with nothing reconciling them or surfacing the gap.
• parseFloat behavior confirmed per the ECMAScript specification and directly reproducible: parseFloat("1,000") → 1; parseFloat("100USD") → 100; parseFloat("50 units") → 50; parseFloat("") → NaN (this last case is caught by the existing !Number.isNaN(amount) check, so purely-empty amounts are excluded — but non-empty, partially-numeric strings are not caught, and are silently mangled instead).
• src/validation/schemas.js:107-122 (recipientsSchema, used to re-validate parsed CSV rows via parseRecipients after parseCSV returns): confirmed amount: z.number().positive() — this schema validates the already-parsed numeric value (e.g. 1 after "1,000" was truncated) for positivity, but has no way to know or object that 1 came from a mangled "1,000" input, since by the time Zod sees it, the truncation has already silently happened inside parseCSV and the original string is gone.
• Confirmed (per the companion merge-conflict issue in this batch) that neither the CSV-upload path nor the JSON-body recipients-array path within the currently-unparseable src/routes/airdrops.js performs any total_amount-vs-sum cross-check for recipients added via POST /airdrops/:id/recipients — only airdropCreateBodySchema's superRefine (the initial POST /airdrops creation path) has that check, and only for inline JSON-supplied recipients, not CSV-derived ones added afterward.

Additional edge cases

• Because total_amount is fixed at airdrop-creation time and recipients-via-CSV are added afterward with no sum-reconciliation against it (see above), a silently-mangled CSV amount wouldn't necessarily be caught by any other layer of validation in this system today — this issue's fix is the most direct, and possibly the only, safety net against this specific class of financial-data corruption on the CSV-upload path specifically.
• Worth considering whether the fix should also validate that amount doesn't exceed some sane per-recipient maximum independent of the eventual total_amount reconciliation (related to, but distinct from, open issue No upper-bound / int64 stroop overflow validation on airdrop total_amount and recipient amounts #73's int64/stroop-overflow concern) — out of scope for this issue's core fix, but worth a mention in the PR discussion since both touch "how much do we trust a numeric amount from user input" in the same recipient-processing pipeline.
• This issue's fix should be coordinated with whoever resolves the unrelated merge-conflict issue in the same file (src/routes/airdrops.js), since both touch the CSV-upload handler; sequencing the two PRs (or combining them) is worth discussing rather than risking one clobbering the other's changes.

Test/reproduction plan

const csvMissingAmount = 'address,amount\nGABC...XYZ,\nGDEF...UVW,100\n';
const csvCommaFormatted = 'address,amount\nGABC...XYZ,"1,000"\n';
const csvTrailingGarbage = 'address,amount\nGABC...XYZ,100USD\n';

// For each: upload via POST /airdrops/:id/recipients with the CSV as `file`.
// Currently: the first silently produces only 1 recipient (the second row) with no error;
// the second silently produces { address: 'GABC...XYZ', amount: 1 } instead of 1000 or a rejection;
// the third silently produces { address: 'GABC...XYZ', amount: 100 } instead of a rejection.
// After fix: all three should be rejected with a clear, row-numbered validation error
// (or, for the comma case only, deliberately and correctly parsed to 1000 if comma-stripping is implemented).

Cross-references

• Airdrop recipient-sum validation uses strict floating-point equality against total_amount #72 (open) — "Airdrop recipient-sum validation uses strict floating-point equality against total_amount." Related (both concern recipient-amount data integrity) but distinct — Airdrop recipient-sum validation uses strict floating-point equality against total_amount #72 is about the creation-time JSON-body path's sum-comparison precision; this issue is about the CSV-upload path's parsing correctness, which has no equivalent sum-check at all today, making this issue's silent-corruption risk arguably more severe on that specific path.
• No upper-bound / int64 stroop overflow validation on airdrop total_amount and recipient amounts #73 (open) — "No upper-bound / int64 stroop overflow validation on airdrop total_amount and recipient amounts." Adjacent concern (numeric-input trustworthiness for recipient amounts); worth a shared review pass.
• No duplicate-recipient protection across multiple POST /airdrops/:id/recipients calls — double-payment risk #81 (open) — "No duplicate-recipient protection across multiple POST /airdrops/:id/recipients calls." Same handler/file; worth sequencing all recipients-upload-path fixes together given how much of this specific route is being touched across multiple open issues.
• Companion issue in this batch — "src/routes/airdrops.js contains an unresolved merge conflict" — this issue's fix necessarily touches the same handler; coordinate the two.

[devkryssie]

I’m interested in this issue because it addresses an important data-integrity problem in the airdrop CSV upload flow. I have experience with TypeScript/JavaScript, backend validation, and API development, and I can implement strict amount parsing, clear row-level validation errors, and comprehensive regression tests. I’m confident I can investigate, implement, test, and submit a clean PR within 2 days.

[grantfox-oss]

🦊 GrantFox — @devkryssie has been assigned to this issue as part of the Third Campaign campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #134)
2. Your PR will be reviewed by the SmartDropLabs maintainers

Good luck! Track your progress on GrantFox.