Skip to content

Fix Bundle authorization with RequestContext - #5827

Draft
adithipo wants to merge 2 commits into
mainfrom
personal/adithip/bug-bundle-context
Draft

adithipo wants to merge 2 commits into
mainfrom
personal/adithip/bug-bundle-context

Conversation

@adithipo

Copy link
Copy Markdown

Description

Pass internal Bundle context through FhirRequestContext.Properties instead of HTTP headers, preventing forged metadata from changing authorization behavior.
Add regression tests for ignored headers and isolated contexts during sequential and parallel Bundle execution.

Related issues

Addresses BUG 206795

Testing

Additional tests and existing tests run locally.

FHIR Team Checklist

  • Update the title of the PR to be succinct and less than 65 characters
  • Add a milestone to the PR for the sprint that it is merged (i.e. add S47)
  • Tag the PR with the type of update: Bug, Build, Dependencies, Enhancement, New-Feature or Documentation
  • Tag the PR with Open source, Azure API for FHIR (CosmosDB or common code) or Azure Healthcare APIs (SQL or common code) to specify where this change is intended to be released.
  • Tag the PR with Schema Version backward compatible or Schema Version backward incompatible or Schema Version unchanged if this adds or updates Sql script which is/is not backward compatible with the code.
  • When changing or adding behavior, if your code modifies the system design or changes design assumptions, please create and include an ADR.
  • CI is green before merge Build Status
  • Review squash-merge requirements

Server Change (docs)

Patch|Skip|Feature|Breaking (reason)

@adithipo adithipo self-assigned this Sep 17, 2026
@adithipo adithipo added the Bug-Security Security related bugs. label Sep 17, 2026
@adithipo adithipo added this to the FY27\Q1\2wk\2wk06 milestone Sep 17, 2026
@adithipo

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree company="Microsoft"

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No unresolved issues were identified that would block approval.

Pull request overview

Fixes Bundle authorization bypass by storing internal execution context in server-side request properties instead of forgeable HTTP headers.

Changes:

  • Updated Bundle context propagation and retrieval.
  • Added sequential/parallel isolation regression tests.
  • Added controller tests confirming headers are ignored.
File summaries
File Description
src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleHandler.cs Updated as part of this pull request.
src/Microsoft.Health.Fhir.Shared.Api/Controllers/FhirController.cs Updated as part of this pull request.
src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Features/Resources/Bundle/BundleHandlerTests.cs Updated as part of this pull request.
src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Controllers/FhirControllerTests.cs Updated as part of this pull request.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@adithipo adithipo added Open source This change is only relevant to the OSS code or release. No-PaaS-breaking-change No-ADR ADR not needed labels Sep 17, 2026
@adithipo adithipo changed the title Fix Bundle authorization bypass by trusting server-side request context Fix Bundle authorization with RequestContext Sep 17, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.66%. Comparing base (66654af) to head (3ae5a34).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5827      +/-   ##
==========================================
+ Coverage   78.16%   78.66%   +0.50%     
==========================================
  Files        1017     1017              
  Lines       36996    36995       -1     
  Branches     5644     5643       -1     
==========================================
+ Hits        28917    29103     +186     
+ Misses       6686     6475     -211     
- Partials     1393     1417      +24     

see 23 files with indirect coverage changes

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug-Security Security related bugs. No-ADR ADR not needed No-PaaS-breaking-change Open source This change is only relevant to the OSS code or release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants