Skip to content

[Shared.SqlProcessor] Add support for handling unterminated escaped identifiers in sanitization#4317

Merged
martincostello merged 4 commits into
open-telemetry:mainfrom
ysolomchenko:Unclosed-escaped-identifiers-can-bypass-SQL-literal-sanitization
Apr 29, 2026
Merged

[Shared.SqlProcessor] Add support for handling unterminated escaped identifiers in sanitization#4317
martincostello merged 4 commits into
open-telemetry:mainfrom
ysolomchenko:Unclosed-escaped-identifiers-can-bypass-SQL-literal-sanitization

Conversation

@ysolomchenko
Copy link
Copy Markdown
Contributor

@ysolomchenko ysolomchenko commented Apr 28, 2026

Fixes # N/A
Design discussion issue # N/A

Found by Codex analysis

Changes

Add support for handling unterminated escaped identifiers in sanitization

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
  • Unit tests added/updated
  • Appropriate CHANGELOG.md files updated for non-trivial changes
  • Changes in public API reviewed (if applicable)

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 28, 2026

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 75.87%. Comparing base (76867f2) to head (f0eb65d).
⚠️ Report is 20 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/Shared/SqlProcessor.cs 88.88% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4317      +/-   ##
==========================================
- Coverage   75.97%   75.87%   -0.10%     
==========================================
  Files         466      466              
  Lines       18564    18572       +8     
==========================================
- Hits        14104    14092      -12     
- Misses       4460     4480      +20     
Flag Coverage Δ
unittests-Contrib.Shared.Tests 89.38% <88.88%> (-0.02%) ⬇️
unittests-Exporter.Geneva 54.82% <ø> (-0.29%) ⬇️
unittests-Exporter.InfluxDB 95.81% <ø> (ø)
unittests-Exporter.Instana 92.00% <ø> (ø)
unittests-Exporter.OneCollector 94.61% <ø> (ø)
unittests-Extensions 90.78% <ø> (ø)
unittests-Extensions.Enrichment 100.00% <ø> (ø)
unittests-Extensions.Enrichment.AspNetCore 86.27% <ø> (ø)
unittests-Extensions.Enrichment.Http 94.33% <ø> (ø)
unittests-Instrumentation.AWS 85.22% <ø> (ø)
unittests-Instrumentation.AspNetCore 70.44% <ø> (ø)
unittests-Instrumentation.Cassandra 92.85% <ø> (ø)
unittests-Instrumentation.ConfluentKafka 78.52% <ø> (ø)
unittests-Instrumentation.ElasticsearchClient 80.60% <ø> (ø)
unittests-Instrumentation.EntityFrameworkCore 81.39% <ø> (ø)
unittests-Instrumentation.EventCounters 77.27% <ø> (ø)
unittests-Instrumentation.GrpcCore 91.27% <ø> (ø)
unittests-Instrumentation.GrpcNetClient 73.78% <ø> (ø)
unittests-Instrumentation.Hangfire 88.91% <ø> (ø)
unittests-Instrumentation.Http 74.62% <ø> (ø)
unittests-Instrumentation.Owin 88.62% <ø> (ø)
unittests-Instrumentation.Process 100.00% <ø> (ø)
unittests-Instrumentation.Quartz 78.76% <ø> (ø)
unittests-Instrumentation.Remoting 64.28% <ø> (ø)
unittests-Instrumentation.Runtime 100.00% <ø> (ø)
unittests-Instrumentation.ServiceFabricRemoting 40.83% <ø> (ø)
unittests-Instrumentation.SqlClient 84.56% <ø> (ø)
unittests-Instrumentation.StackExchangeRedis 93.63% <ø> (ø)
unittests-Instrumentation.Wcf 81.52% <ø> (+0.13%) ⬆️
unittests-OpAmp.Client 84.46% <ø> (ø)
unittests-PersistentStorage 68.81% <ø> (-1.61%) ⬇️
unittests-Resources.AWS 74.49% <ø> (ø)
unittests-Resources.Azure 88.31% <ø> (ø)
unittests-Resources.Container 67.34% <ø> (ø)
unittests-Resources.Gcp 71.42% <ø> (ø)
unittests-Resources.Host 72.26% <ø> (ø)
unittests-Resources.OperatingSystem 76.98% <ø> (ø)
unittests-Resources.Process 100.00% <ø> (ø)
unittests-Resources.ProcessRuntime 79.59% <ø> (ø)
unittests-Sampler.AWS 97.52% <ø> (ø)

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

Files with missing lines Coverage Δ
src/Shared/SqlProcessor.cs 98.90% <88.88%> (-0.17%) ⬇️

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/Shared/SqlProcessor.cs
Comment thread test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorTests.cs
Copy link
Copy Markdown
Member

@Kielek Kielek left a comment

Choose a reason for hiding this comment

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

I think that it deserve changelog entry.
SqlClient + EFCore as I rembmer/

Comment thread test/OpenTelemetry.Contrib.Shared.FuzzTests/SqlProcessorTests.cs Outdated
Comment thread test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorTests.cs Outdated
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Apr 28, 2026

CLA Signed

The committers listed above are authorized under a signed CLA.

 format invariantly

Changelogs
@github-actions github-actions Bot added comp:instrumentation.entityframeworkcore Things related to OpenTelemetry.Instrumentation.EntityFrameworkCore comp:instrumentation.sqlclient Things related to OpenTelemetry.Instrumentation.SqlClient labels Apr 28, 2026
@ysolomchenko ysolomchenko force-pushed the Unclosed-escaped-identifiers-can-bypass-SQL-literal-sanitization branch from 7628da4 to f0eb65d Compare April 28, 2026 12:32
@github-actions github-actions Bot requested a review from matt-hensley April 28, 2026 12:32
@martincostello martincostello added this pull request to the merge queue Apr 29, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 29, 2026
@martincostello martincostello added this pull request to the merge queue Apr 29, 2026
Merged via the queue into open-telemetry:main with commit 052ddf2 Apr 29, 2026
316 of 317 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:instrumentation.entityframeworkcore Things related to OpenTelemetry.Instrumentation.EntityFrameworkCore comp:instrumentation.sqlclient Things related to OpenTelemetry.Instrumentation.SqlClient

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants