Skip to content

Add SQL injection prevention audit (#777) - #863

Open
Hollujay wants to merge 4 commits into
ritik4ever:mainfrom
Hollujay:feat/777-sql-injection-audit
Open

Add SQL injection prevention audit (#777)#863
Hollujay wants to merge 4 commits into
ritik4ever:mainfrom
Hollujay:feat/777-sql-injection-audit

Conversation

@Hollujay

Copy link
Copy Markdown
Contributor

Summary

Closes #777

Audited every .prepare() / .exec() call site in backend/src/services/db.ts, backend/src/index.ts, and the rest of backend/src/services/ (58 call sites total) for raw string interpolation of user-controlled input.

Finding: zero raw string interpolation of untrusted data. Every query already uses ? positional or @name named bind parameters (per this repo's own CLAUDE.md "Code Patterns" guidance). The only template-literal interpolations in the codebase are closed, non-attacker-controlled tokens:

  • ORDER BY column name, sourced from a SORT_COLUMNS allowlist keyed by a typed SortField union (streamStore.ts)
  • ORDER BY direction, always the literal result of a === 'asc' ? 'ASC' : 'DESC' ternary
  • INDEXER_CURSOR_TABLE, a module-level constant, never derived from input

Full findings are documented in SQL_INJECTION_AUDIT.md.

Changes

  • SQL_INJECTION_AUDIT.md — audit methodology, call-site inventory, and findings table.
  • backend/src/services/sqlInjection.integration.test.ts — regression tests that feed classic SQLi payloads (' OR '1'='1, '; DROP TABLE streams; --, ' UNION SELECT * FROM streams --, etc.) through every user-reachable string input in listStreamsByRecipient, listStreamsBySender, getStreamHistory, getGlobalEvents, and countAllEvents, asserting the payloads are stored/matched as inert plain text and the schema/data stays intact.

Test plan

  • New tests follow this repo's existing integration-test pattern (real SQLite file via initDb()/getDb(), cleaned per-test, closed and deleted in afterAll), matching streamStore.cancel.integration.test.ts.
  • Note: I was unable to run npm run test in this environment (node_modules isn't installed and package installation is blocked here), so these tests are unverified by a local run — please run CI/npx vitest run src/services/sqlInjection.integration.test.ts to confirm before merging.

…ritik4ever#777)

Audited every .prepare()/.exec() call site in backend/src/services and
index.ts. All user-controlled input is passed via ? or @name bound
parameters; the only template-literal interpolations found are closed
allowlisted tokens (ORDER BY column/direction, a constant table name),
none of which are attacker-controlled.

Adds sqlInjection.integration.test.ts with classic SQLi payloads run
through listStreamsByRecipient/BySender, getStreamHistory,
getGlobalEvents, and countAllEvents to lock this in as a regression
test, plus SQL_INJECTION_AUDIT.md documenting the findings.
@vercel

vercel Bot commented Aug 28, 2026

Copy link
Copy Markdown

@Hollujay is attempting to deploy a commit to the ritik4ever's projects Team on Vercel.

A member of the Team first needs to authorize it.

@drips-wave

drips-wave Bot commented Aug 28, 2026

Copy link
Copy Markdown

@Hollujay 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! 🚀

Learn more about application limits

@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ee7cb486-a02b-48ee-9dd8-8f355c88ad2e


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

- streamStore.ts: remove dangling TransactionBuilder fragment left by
  a botched merge, which caused TS1005/TS1128 syntax errors and broke
  the backend type-check job
- frontend: regenerate package-lock.json/package.json so `npm ci`
  succeeds again (was failing with EUSAGE on a stale lockfile),
  fixing the Playwright E2E job
- carries the rest of the in-progress main-build restoration
  (db.ts FTS/allowed-assets support, migration cleanup, validateEnv,
  streamStore fixes, test fixes)
Hollujay added a commit to Hollujay/stellar-stream that referenced this pull request Aug 28, 2026
- streamStore.ts: remove dangling TransactionBuilder fragment left by
  a botched merge, which caused TS1005/TS1128 syntax errors and broke
  the backend type-check job
- frontend: regenerate package-lock.json/package.json so `npm ci`
  succeeds again (was failing with EUSAGE on a stale lockfile),
  fixing the Playwright E2E job
- carries the rest of the in-progress main-build restoration
  (db.ts FTS/allowed-assets support, migration cleanup, validateEnv,
  streamStore fixes, test fixes)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KjWaRADoMHAnnth3NCoNsL
…dep, stale Node pin

- backend: type the genuine `any`s in websocket.ts/sorobanRetry.ts; extend
  eslint overrides for db.ts (this-alias, Function type) and migrations.ts
  (duck-typed db param), and allow require() in test files, matching the
  project's existing override pattern
- frontend: fix a real Rules-of-Hooks violation in CliffMarker.tsx
  (useMemo called after an early return); drop unused imports/vars in
  SenderDashboard.tsx and StreamsTable.stories.tsx; rename Storybook
  render functions to PascalCase components so hooks inside them are
  recognized; allow console in *.stories.tsx (mock action handlers)
- frontend: add missing rollup-plugin-visualizer devDependency that
  vite.config.ts already imported, unblocking the Lighthouse CI build
- playwright-e2e.yml: bump Node 18 -> 20; the installed Playwright
  version requires Node 20+
Hollujay added a commit to Hollujay/stellar-stream that referenced this pull request Aug 28, 2026
…dep, stale Node pin

- backend: type the genuine `any`s in websocket.ts/sorobanRetry.ts; extend
  eslint overrides for db.ts (this-alias, Function type) and migrations.ts
  (duck-typed db param), and allow require() in test files, matching the
  project's existing override pattern
- frontend: fix a real Rules-of-Hooks violation in CliffMarker.tsx
  (useMemo called after an early return); drop unused imports/vars in
  SenderDashboard.tsx and StreamsTable.stories.tsx; rename Storybook
  render functions to PascalCase components so hooks inside them are
  recognized; allow console in *.stories.tsx (mock action handlers)
- frontend: add missing rollup-plugin-visualizer devDependency that
  vite.config.ts already imported, unblocking the Lighthouse CI build
- playwright-e2e.yml: bump Node 18 -> 20; the installed Playwright
  version requires Node 20+

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KjWaRADoMHAnnth3NCoNsL
- add missing @vitest/coverage-v8 devDependency (vitest --coverage was
  crashing at startup with 'Cannot find dependency')
- set CI=true in frontend/Dockerfile's builder stage so the Docker
  build hits the same vite-plugin-pwa skip that GitHub Actions gets,
  avoiding the workbox-build validate-options crash
@Hollujay
Hollujay force-pushed the feat/777-sql-injection-audit branch from feda982 to c48cb03 Compare August 28, 2026 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Add SQL injection prevention audit

1 participant