Skip to content

fix: fix some security review suggestions#485

Merged
wschurman merged 1 commit intomainfrom
wschurman/02-27-fix_fix_some_security_issues
Feb 28, 2026
Merged

fix: fix some security review suggestions#485
wschurman merged 1 commit intomainfrom
wschurman/02-27-fix_fix_some_security_issues

Conversation

@wschurman
Copy link
Member

@wschurman wschurman commented Feb 27, 2026

Why

Had claude do a security review of entity-database-adapter-knex.

How

The items it noticed were:

  • orderBySpecification.order is type safe but could be overridden via an any-casted value, which could result in an injection since it was directly inserted into the knex orderByRaw method. The fix is to do a runtime check for value equal to the enum.
  • Better type constraints for like/iLike/etc since they should only be usable for string-like fields.
  • For jsonContains, need to constrain types and do a runtime validity check to ensure value is JSON-serializable.

Test Plan

Run new tests.

Copy link
Member Author

wschurman commented Feb 27, 2026

@codecov
Copy link

codecov bot commented Feb 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (459a5ff) to head (deb3741).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #485   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          110       110           
  Lines        16365     16394   +29     
  Branches       884       900   +16     
=========================================
+ Hits         16365     16394   +29     
Flag Coverage Δ
integration 24.69% <51.21%> (+0.06%) ⬆️
unittest 94.85% <92.68%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@wschurman wschurman force-pushed the wschurman/02-27-fix_fix_some_security_issues branch from ebd68b8 to 324a609 Compare February 27, 2026 17:30
@wschurman wschurman force-pushed the wschurman/02-26-feat_add_entityfield_sql_helper branch from 37e8024 to ef18615 Compare February 27, 2026 17:30
@wschurman wschurman changed the title fix: fix some security issues fix: fix some security review suggestions Feb 27, 2026
@wschurman wschurman force-pushed the wschurman/02-27-fix_fix_some_security_issues branch from 324a609 to 1e06471 Compare February 27, 2026 17:49
@wschurman wschurman requested review from ide and quinlanj February 27, 2026 17:52
@wschurman wschurman marked this pull request as ready for review February 27, 2026 18:57
@wschurman wschurman changed the base branch from wschurman/02-26-feat_add_entityfield_sql_helper to graphite-base/485 February 27, 2026 23:41
@wschurman wschurman requested a review from ide February 28, 2026 00:11
@wschurman wschurman force-pushed the wschurman/02-27-fix_fix_some_security_issues branch from 1e06471 to 1594f42 Compare February 28, 2026 00:11
@wschurman wschurman changed the base branch from graphite-base/485 to main February 28, 2026 00:11
@wschurman wschurman force-pushed the wschurman/02-27-fix_fix_some_security_issues branch from 1594f42 to deb3741 Compare February 28, 2026 00:52
@wschurman wschurman merged commit e263bd2 into main Feb 28, 2026
5 checks passed
@wschurman wschurman deleted the wschurman/02-27-fix_fix_some_security_issues branch February 28, 2026 01:59
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.

3 participants