Skip to content

Use a query parameter for the subscription run flag - #285

Merged
hamzahalq merged 2 commits into
releases/r8.0from
hamza/fix/parameterize-runflag-sql
Sep 3, 2026
Merged

Use a query parameter for the subscription run flag#285
hamzahalq merged 2 commits into
releases/r8.0from
hamza/fix/parameterize-runflag-sql

Conversation

@hamzahalq

Copy link
Copy Markdown
Contributor
  • Binds the subscription id as a real query parameter in RunFlagUpdater instead of interpolating it into the SQL, which also drops the bogus quotes around an integer.
  • Adds RunFlagUpdaterTests, this code had no coverage.

Not exploitable (the value is a C# int), but it is the pattern SAST tools flag as SQL injection, and it makes our written answer on the security assessment accurate.

Verified: integration suite 73/73 passes, and the EF command log shows WHERE id = @p0 with the value bound separately.

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.
@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (8)
  • ^main$
  • ^master$
  • ^staging$
  • ^development$
  • ^gigstaging$
  • ^develop$
  • ^releases$
  • ^releases/r10.0$

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: simplify9/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 629d5d0b-ff70-4f28-8395-dadf446fbf93

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: simplify9/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 001abadd-5d8c-4af7-ba72-5e56f4cd88b7

📥 Commits

Reviewing files that changed from the base of the PR and between 7131283 and 1306252.

📒 Files selected for processing (2)
  • SW.Bitween.Api/Services/RunFlagUpdater.cs
  • SW.Bitween.IntegrationTests/Tests/RunFlagUpdaterTests.cs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details
🔇 Additional comments (2)
SW.Bitween.Api/Services/RunFlagUpdater.cs (1)

24-38: LGTM!

Also applies to: 49-59

SW.Bitween.IntegrationTests/Tests/RunFlagUpdaterTests.cs (1)

1-72: LGTM!


📝 Summary

What changed

  • RunFlagUpdater now binds the subscription ID as a SQL parameter in MarkAsRunning and MarkAsIdle.
  • Added integration tests for claim-once behavior, idle re-claiming, and subscription isolation.

Risk

risk:low

Security-sensitive areas

  • Removes direct SQL interpolation of the subscription ID.
  • Prevents the SQL injection pattern flagged by SAST tools.

Test coverage impact

  • Added RunFlagUpdaterTests.
  • Integration coverage now verifies the main run-flag state transitions and update scope.
  • All 73 integration tests pass.

Operational concerns

  • No migration or deployment changes are required.
  • Rollback only requires reverting the application code.
  • EF logging confirms parameterized SQL using WHERE id = @p0``.

Walkthrough

Changes

Run Flag Safety

Layer / File(s) Summary
Parameterized run flag updates
SW.Bitween.Api/Services/RunFlagUpdater.cs
MarkAsRunning and MarkAsIdle use {0} SQL placeholders and pass id as a parameter.
Run flag integration validation
SW.Bitween.IntegrationTests/Tests/RunFlagUpdaterTests.cs
Integration tests cover single-claim behavior, idle transitions, reclaiming, and isolation between subscriptions.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 13062

Run-flag updates now bind subscription IDs as SQL parameters while retaining claim, idle, reclaim, and subscription-isolation behavior. The change is covered by integration tests and is ready to merge.

Suggested labels: security, database, testing, risk:high

Suggested reviewers: mmalkhatib

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the primary change: parameterizing the subscription run flag query.
Description check ✅ Passed The description accurately covers query parameter binding, SQL injection pattern removal, added integration tests, and verification results.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@hamzahalq hamzahalq closed this Sep 3, 2026
@hamzahalq hamzahalq reopened this Sep 3, 2026
@hamzahalq
hamzahalq merged commit 0292c02 into releases/r8.0 Sep 3, 2026
3 checks passed
@hamzahalq
hamzahalq deleted the hamza/fix/parameterize-runflag-sql branch September 3, 2026 09:42
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