From 1306252eb8e40bfb9df61b327d85c4083737a1c9 Mon Sep 17 00:00:00 2001 From: Hamza Alqurneh Date: Thu, 3 Sep 2026 12:08:02 +0300 Subject: [PATCH 1/2] Use a query parameter for the subscription run flag The run-flag statements interpolated the id straight into the SQL. It was not exploitable, the value is an int, but it is the pattern SAST tools flag as SQL injection. Bind it as a parameter instead, and add the missing tests. --- SW.Bitween.Api/Services/RunFlagUpdater.cs | 30 ++++---- .../Tests/RunFlagUpdaterTests.cs | 72 +++++++++++++++++++ 2 files changed, 87 insertions(+), 15 deletions(-) create mode 100644 SW.Bitween.IntegrationTests/Tests/RunFlagUpdaterTests.cs diff --git a/SW.Bitween.Api/Services/RunFlagUpdater.cs b/SW.Bitween.Api/Services/RunFlagUpdater.cs index 5a1094d7..4b377d87 100644 --- a/SW.Bitween.Api/Services/RunFlagUpdater.cs +++ b/SW.Bitween.Api/Services/RunFlagUpdater.cs @@ -21,21 +21,21 @@ public async Task MarkAsRunning(int id) { var sqlUpdate = _dbType.ToLower() switch { - "pgsql" => $@"UPDATE infolink.subscription SET is_running = true - WHERE id = '{id}' and is_running = false + "pgsql" => @"UPDATE infolink.subscription SET is_running = true + WHERE id = {0} and is_running = false RETURNING is_running", - "mssql" => $@"UPDATE Subscriptions SET IsRunning = 1 + "mssql" => @"UPDATE Subscriptions SET IsRunning = 1 OUTPUT INSERTED.IsRunning - WHERE Id = '{id}' and IsRunning = 0", - "mysql" => $@"SELECT IsRunning FROM Subscriptions - WHERE Id = '{id}' and IsRunning = false + WHERE Id = {0} and IsRunning = 0", + "mysql" => @"SELECT IsRunning FROM Subscriptions + WHERE Id = {0} and IsRunning = false FOR UPDATE; UPDATE Subscriptions SET IsRunning = true - WHERE Id = '{id}' and IsRunning = false", + WHERE Id = {0} and IsRunning = false", _ => "" }; - var results = await dbContext.Set().FromSqlRaw(sqlUpdate).ToListAsync(); + var results = await dbContext.Set().FromSqlRaw(sqlUpdate, id).ToListAsync(); var result = results.SingleOrDefault(); // result is null when is running is true @@ -46,17 +46,17 @@ public async Task MarkAsIdle(int id) { var sqlUpdate = _dbType.ToLower() switch { - "pgsql" => $@"UPDATE infolink.subscription SET is_running = false - WHERE id = '{id}'", - "mssql" => $@"UPDATE Subscriptions SET IsRunning = 0 - WHERE Id = '{id}'", - "mysql" => $@"UPDATE Subscriptions SET IsRunning = false - WHERE Id = '{id}'", + "pgsql" => @"UPDATE infolink.subscription SET is_running = false + WHERE id = {0}", + "mssql" => @"UPDATE Subscriptions SET IsRunning = 0 + WHERE Id = {0}", + "mysql" => @"UPDATE Subscriptions SET IsRunning = false + WHERE Id = {0}", _ => "" }; ; - await dbContext.Database.ExecuteSqlRawAsync(sqlUpdate); + await dbContext.Database.ExecuteSqlRawAsync(sqlUpdate, id); } public class RunningResult diff --git a/SW.Bitween.IntegrationTests/Tests/RunFlagUpdaterTests.cs b/SW.Bitween.IntegrationTests/Tests/RunFlagUpdaterTests.cs new file mode 100644 index 00000000..079b85f8 --- /dev/null +++ b/SW.Bitween.IntegrationTests/Tests/RunFlagUpdaterTests.cs @@ -0,0 +1,72 @@ +using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; +using SW.Bitween.Domain; +using SW.Bitween.IntegrationTests.Fixtures; +using SW.Bitween.Model; +using Xunit; + +namespace SW.Bitween.IntegrationTests.Tests; + +// The run flag is written with raw (parameterized) SQL that differs per database +// provider, so it needs a real round trip to prove the statement and its parameter +// binding are correct. There was no coverage here before. +[Collection("Bitween")] +public class RunFlagUpdaterTests +{ + private readonly BitweenFixture _fixture; + + public RunFlagUpdaterTests(BitweenFixture fixture) + { + _fixture = fixture; + } + + [Fact] + public async Task Run_flag_claims_once_then_blocks_until_idle() + { + await using var scope = _fixture.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var runFlag = scope.ServiceProvider.GetRequiredService(); + + var document = new Document(6101, "Run Flag Test Doc"); + db.Set().Add(document); + var subscription = new Subscription("Run Flag Test", document.Id); + subscription.Inactive = false; + db.Set().Add(subscription); + await db.SaveChangesAsync(); + + Assert.True(await runFlag.MarkAsRunning(subscription.Id)); // first claim wins + Assert.False(await runFlag.MarkAsRunning(subscription.Id)); // already running + + await runFlag.MarkAsIdle(subscription.Id); + + Assert.True(await runFlag.MarkAsRunning(subscription.Id)); // claimable again + await runFlag.MarkAsIdle(subscription.Id); + } + + // Guards the parameter binding: a broken placeholder would either match no rows + // or every row, and both would show up here. + [Fact] + public async Task Run_flag_only_affects_the_requested_subscription() + { + await using var scope = _fixture.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var runFlag = scope.ServiceProvider.GetRequiredService(); + + var document = new Document(6102, "Run Flag Isolation Doc"); + db.Set().Add(document); + var a = new Subscription("Run Flag A", document.Id); + a.Inactive = false; + var b = new Subscription("Run Flag B", document.Id); + b.Inactive = false; + db.Set().AddRange(a, b); + await db.SaveChangesAsync(); + + Assert.True(await runFlag.MarkAsRunning(a.Id)); + Assert.True(await runFlag.MarkAsRunning(b.Id)); // b untouched by a's update + + await runFlag.MarkAsIdle(a.Id); + Assert.False(await runFlag.MarkAsRunning(b.Id)); // b still running, a's idle did not clear it + + await runFlag.MarkAsIdle(b.Id); + } +} From ed38402a3331dc44c359465fe6d4d15e5f12c431 Mon Sep 17 00:00:00 2001 From: Musa Misto <64855513+MusaMisto@users.noreply.github.com> Date: Thu, 3 Sep 2026 12:41:30 +0300 Subject: [PATCH 2/2] Update branches for vuln-check workflow --- .github/workflows/critical-vuln-check.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/critical-vuln-check.yml b/.github/workflows/critical-vuln-check.yml index 61aa59f6..70f912b7 100644 --- a/.github/workflows/critical-vuln-check.yml +++ b/.github/workflows/critical-vuln-check.yml @@ -22,7 +22,7 @@ run-name: vuln-check-${{ github.event.pull_request.number }} on: pull_request_target: - branches: [releases/r8.0, develop] + branches: [releases/r10.0, releases/r8.0, develop] # critical-vuln-gate.yml's own job requests these two scopes -- a caller can # only narrow permissions for a nested reusable-workflow job, never widen