fix: address security vulnerabilities and pagination bugs by marcelosalloum · Pull Request #60 · stellar/laboratory-backend

marcelosalloum · 2026-04-14T20:02:34Z

What

Encode XDR binary data as base64 in serializer (BREAKING: key and value fields)
Add point-of-use validation in orderBy() for sortDbField and direction
Warn on unanchored CORS_ORIGINS regex patterns
Handle NULL sort column values in cursor-based pagination (NULLS LAST/FIRST, IS NULL conditions)
Write Zod-parsed values to res.locals so controllers use coerced types; replace parseInt with Number()
Preserve sub-second timestamp precision in pagination cursors
Encode and validate sortDirection in cursors to prevent order flipping
Add truncated flag to keys endpoint response
Prevent encodeCursor from mutating its input object
Add stale-cache fallback (10s window) for Stellar RPC outages

Why

Addresses findings from our automated security and bug finders:

Closes stellar/internal-agents#325
Closes stellar/internal-agents#314
Closes stellar/internal-agents#301
Closes stellar/internal-agents#280
Closes stellar/internal-agents#272
Closes stellar/internal-agents#271
Closes stellar/internal-agents#270

See docs/vulnerability-report.md for the full consolidated report with cross-references to each source issue.

Buffer.from(row.key).toString() defaulted to UTF-8, silently corrupting XDR bytes 0x80-0xFF with Unicode replacement characters. Switching to explicit base64 encoding preserves binary integrity through JSON. BREAKING CHANGE: `key` and `value` fields in /api/contract/:id/storage responses are now base64-encoded instead of raw UTF-8 strings. Ref: stellar/internal-agents#314 Finding 1

…polation Defense-in-depth: validate sortDbField against the allowlist and direction against the SortDirection enum inside orderBy(), right where they are interpolated into raw SQL via Prisma.raw(). The upstream assertValidSortDbField() call in buildContractDataQuery() already guards this path, but a future refactor could bypass it. Ref: stellar/internal-agents#325 NM-002, NM-007 Ref: stellar/internal-agents#301 NM-003

Unanchored regex like /example\.com/ would match evil-example.com. Now logs a console.warn at startup if a CORS_ORIGINS regex is missing ^ or $ anchors, alerting operators to the misconfiguration risk. Ref: stellar/internal-agents#301 NM-007

- extractSortValue converts null to undefined - Cursor schema accepts undefined sortValue for non-key_hash sort fields - ORDER BY includes NULLS LAST (ASC) / NULLS FIRST (DESC) - Cursor conditions include OR IS NULL for non-NULL cursors in ASC scans - New queryWithCursorNullSortField handles navigation from NULL boundaries

The middleware validated with Zod but discarded the result; the controller re-parsed raw query strings with parseInt, which mishandles scientific notation (parseInt("1e2") = 1 instead of 100). Now the middleware writes parsed.data back and the controller uses Number() with consistent defaults.

Math.floor(getTime()/1000) truncated milliseconds, causing infinite pagination loops when records shared the same second but differed in sub-second precision. Keeping fractional seconds allows to_timestamp() to reconstruct the exact boundary.

Cursors now include the sort direction. Reusing a cursor with a different order parameter returns 400. Old cursors without sortDirection are still accepted for backward compatibility.

Spread into a new object before converting bigint sortValue to string, so the caller's original CursorData remains unchanged.

When fetchLatestLedger fails after cache TTL, return the stale cached value if it is less than 10 seconds old. Beyond that window the error propagates normally so callers are not silently served arbitrarily old data.

When the recursive CTE hits the 10,000 key limit, total_keys is misleading. The new truncated boolean lets callers know the result set is incomplete.

Resolves moderate vulnerability GHSA-r4q5-vmmm-2653 (auth header leak on cross-domain redirects). The existing axios ^1.15.11 range already allows 1.16.0; only the lockfile was pinned to the vulnerable version.

Copilot

Pull request overview

This PR hardens the contract-data and keys endpoints against several security/pagination edge cases (cursor tampering, NULL sort behavior, binary serialization), while improving runtime validation and resilience to Stellar RPC outages.

Changes:

Base64-encode binary contract key/value fields and adjust tests/seed data accordingly.
Strengthen cursor-based pagination: preserve sub-second timestamps, encode sort direction into cursors, and handle NULL sort columns predictably.
Add operational hardening: stale-cache fallback for latest-ledger fetches and warnings for risky (unanchored) CORS regex config.

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/utils/stellar.test.ts	Adds tests for stale-cache fallback behavior when RPC calls fail.
tests/test-data-seeder.ts	Extends seeded contract_data rows to cover NULL TTL and sub-second timestamps.
tests/serializers/contract_data.test.ts	New serializer tests validating base64 encoding and null-handling.
tests/helpers/cursor.test.ts	New test ensuring `encodeCursor` does not mutate its input.
tests/controllers/keys.test.ts	Updates expected response shape to include `truncated`.
tests/controllers/contract_data.test.ts	Expands pagination and serialization assertions (NULL TTL, base64, cursor/order mismatch, record counts).
tests/config/env.test.ts	Adds tests for warning on unanchored `CORS_ORIGINS` regex patterns.
src/utils/stellar.ts	Implements stale-cache fallback window for latest-ledger fetch failures.
src/serializers/contract_data.ts	Changes binary serialization for `key`/`val` to base64 strings.
src/routes/contract_data.ts	Stores Zod-parsed params/query into `res.locals` for downstream use.
src/query-builders/contract_data.ts	Adds sort-field validation, NULLS FIRST/LAST ordering, and cursor logic for NULL sort values.
src/pagination/contract_data.ts	Preserves sub-second precision in timestamp cursors; encodes `sortDirection` into cursors; treats NULL sort values as missing cursor sortValue.
src/helpers/cursor.ts	Adds optional `sortDirection` to cursor schema; prevents `encodeCursor` from mutating input; relaxes sortValue requirement to support NULL boundary cursors.
src/controllers/keys.ts	Adds `truncated` flag in keys response.
src/controllers/contract_data.ts	Consumes Zod-parsed query values, switches to `Number()` parsing, and validates cursor sortDirection vs request order.
src/config/env.ts	Warns when `CORS_ORIGINS` regex is not anchored with `^...$`.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/controllers/contract_data.ts

marcelosalloum · 2026-04-14T20:17:26Z

src/controllers/contract_data.ts

+    if (
+      cursorData.sortDirection !== undefined &&
+      cursorData.sortDirection !== sortDirection
+    ) {
+      throw new CursorParameterMismatchError(
+        "order",
+        sortDirection,
+        cursorData.sortDirection,
+      );
+    }


Intentional — old cursors won't have sortDirection, so requiring it would break existing pagination links in the wild.

src/controllers/contract_data.ts

src/controllers/keys.ts

jeesunikim · 2026-04-14T23:13:25Z

src/controllers/contract_data.ts

-        `Invalid limit=${limit}, must be an integer between 1 and 200`,
-      );
-    }
+  const parsedLimit = typeof limit === "number" ? limit : Number(limit);


looking at #301 NM-005, we can technically remove this since it's validated on routes level right? @marcelosalloum

jeesunikim

added some questions. do we need to address NM 001/006 in #301?

jeesunikim · 2026-04-14T23:21:12Z

src/controllers/keys.ts

  return res.status(200).json({
    contract_id,
    total_keys: keys.length,
+    truncated: keys.length >= MAX_KEYS,


should frontend display something if it's truncated @marcelosalloum

marcelosalloum added 10 commits April 14, 2026 13:01

fix: encode and validate sortDirection in pagination cursors

f912fa3

Cursors now include the sort direction. Reusing a cursor with a different order parameter returns 400. Old cursors without sortDirection are still accepted for backward compatibility.

fix: prevent encodeCursor from mutating its input object

8fe5460

Spread into a new object before converting bigint sortValue to string, so the caller's original CursorData remains unchanged.

fix: return stale cached ledger on transient fetch failures

ae92da1

When fetchLatestLedger fails after cache TTL, return the stale cached value if it is less than 10 seconds old. Beyond that window the error propagates normally so callers are not silently served arbitrarily old data.

fix: add truncated flag to keys endpoint response

f5d9a4d

When the recursive CTE hits the 10,000 key limit, total_keys is misleading. The new truncated boolean lets callers know the result set is incomplete.

github-project-automation bot added this to DevX Apr 14, 2026

github-project-automation bot moved this to Backlog (Not Ready) in DevX Apr 14, 2026

marcelosalloum self-assigned this Apr 14, 2026

marcelosalloum requested a review from jeesunikim April 14, 2026 20:03

marcelosalloum marked this pull request as ready for review April 14, 2026 20:05

Copilot AI review requested due to automatic review settings April 14, 2026 20:05

Copilot started reviewing on behalf of marcelosalloum April 14, 2026 20:06 View session

fix(deps): update follow-redirects to 1.16.0 via lockfile refresh

82be7de

Resolves moderate vulnerability GHSA-r4q5-vmmm-2653 (auth header leak on cross-domain redirects). The existing axios ^1.15.11 range already allows 1.16.0; only the lockfile was pinned to the vulnerable version.

Copilot AI reviewed Apr 14, 2026

View reviewed changes

fix: reject non-integer limit values and update JSDoc

fb3239e

jeesunikim reviewed Apr 14, 2026

View reviewed changes

jeesunikim approved these changes Apr 14, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: address security vulnerabilities and pagination bugs#60

fix: address security vulnerabilities and pagination bugs#60
marcelosalloum wants to merge 12 commits intomainfrom
fix/security-and-bug-fixes

marcelosalloum commented Apr 14, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

marcelosalloum Apr 14, 2026

Uh oh!

Uh oh!

Uh oh!

jeesunikim Apr 14, 2026 •

edited

Loading

Uh oh!

jeesunikim left a comment

Uh oh!

jeesunikim Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

marcelosalloum commented Apr 14, 2026

What

Why

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

marcelosalloum Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jeesunikim Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeesunikim left a comment

Choose a reason for hiding this comment

Uh oh!

jeesunikim Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jeesunikim Apr 14, 2026 •

edited

Loading