Skip to content

Protect operations database from non-string data in error messages fr…#1716

Open
davecrighton wants to merge 2 commits intohyperledger:mainfrom
davecrighton:djc/protectDBAccess
Open

Protect operations database from non-string data in error messages fr…#1716
davecrighton wants to merge 2 commits intohyperledger:mainfrom
davecrighton:djc/protectDBAccess

Conversation

@davecrighton
Copy link

@davecrighton davecrighton commented Feb 27, 2026

Proposed changes

This change tests the error string before making an update to the operations table checking that it is a valid UTF-8 string and does not contain null bytes. The second check is required because UTF-8 considers a single null byte to be a valid string but Postgres does not.

Fixes #1717


Types of changes

  • Bug fix
  • New feature added
  • Documentation Update

Please make sure to follow these points

  • I have read the contributing guidelines.
  • I have performed a self-review of my own code or work.
  • I have commented my code, particularly in hard-to-understand areas.
  • My changes generates no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • My changes have sufficient code coverage (unit, integration, e2e tests).

Screenshots (If Applicable)


Other Information

Any message for the reviewer or kick off the discussion by explaining why you considered this particular solution, any alternatives etc.

…om reverts

Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
@davecrighton davecrighton marked this pull request as ready for review February 27, 2026 15:28
@davecrighton davecrighton requested a review from a team as a code owner February 27, 2026 15:28
@codecov
Copy link

codecov bot commented Feb 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.96%. Comparing base (df3fe81) to head (23f87c2).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1716   +/-   ##
=======================================
  Coverage   99.96%   99.96%           
=======================================
  Files         342      342           
  Lines       25022    25027    +5     
=======================================
+ Hits        25014    25019    +5     
  Misses          4        4           
  Partials        4        4           

☔ 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.

Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
@davecrighton davecrighton force-pushed the djc/protectDBAccess branch from 765a1c5 to fe2e153 Compare March 2, 2026 11:21
update = update.Set("error", hexString)
} else {
update = update.Set("error", *errorMsg)
}
Copy link
Contributor

@peterbroadhurst peterbroadhurst Mar 4, 2026

Choose a reason for hiding this comment

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

Can you make this a utility function please, in a place that can be used across packages.

Then this line becomes:

Suggested change
}
update = update.Set("error", utils.DBSafeStringFromPtr(errorMsg))

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.

bug: Contracts which return a nested error cause failure to insert into operations DB

2 participants