Skip to content

security: fix remaining HIGH findings (H2 remaining, H8, H10)#5600

Merged
chitalian merged 1 commit intoHelicone:mainfrom
ink-the-squid:ink/security-fixes-batch-3
Feb 21, 2026
Merged

security: fix remaining HIGH findings (H2 remaining, H8, H10)#5600
chitalian merged 1 commit intoHelicone:mainfrom
ink-the-squid:ink/security-fixes-batch-3

Conversation

@ink-the-squid
Copy link
Contributor

Security Fixes — Batch 3

H2 (remaining): LIMIT/OFFSET SQL injection hardening

Files: adminController, MetricsManager, HeliconeDatasetManager

  • Validate and clamp limit and offset as safe integers before SQL interpolation
  • Adds Math.floor() + Math.max()/Math.min() bounds

H8: Remove query parameters from error logs

Files: dbExecute.ts, ClickhouseWrapper.ts (jawn + worker), valhalla.ts

  • Removed parameters from console.error() calls in catch blocks
  • Query text is still logged for debugging, but parameter values (which may contain PII, secrets, or user data) are no longer exposed

H10: Replace blanket public route auth bypass with whitelist

File: valhalla/jawn/src/middleware/auth.ts

  • Changed req.path.startsWith('/v1/public') to explicit whitelist of 8 known public route prefixes
  • Prevents future endpoints accidentally added under /v1/public/ from being unauthenticated

All TypeScript compiles clean (only pre-existing Gemini duplicate import errors on main).

H2 (remaining): Validate/clamp LIMIT and OFFSET as safe integers in
    adminController, MetricsManager, and HeliconeDatasetManager.

H8: Remove query parameters from error log statements in
    dbExecute, ClickhouseWrapper (jawn + worker), and valhalla.
    Prevents PII/secrets from leaking into logs.

H10: Replace blanket /v1/public/* auth bypass with explicit
    whitelist of known public route prefixes. Prevents accidental
    exposure of new endpoints added under /v1/public/.
@vercel
Copy link

vercel bot commented Feb 21, 2026

@ink-the-squid is attempting to deploy a commit to the Helicone Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@chitalian chitalian merged commit 3351391 into Helicone:main Feb 21, 2026
6 of 10 checks passed
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.

2 participants