Skip to content

[PM-34909] Serve only validated report files from get-latest and self-hosted file endpoints #7465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AlexRubik wants to merge 15 commits into
base: main
Choose a base branch
from
+435 −14
Open

[PM-34909] Serve only validated report files from get-latest and self-hosted file endpoints #7465

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
091d503
feat(dirt): enforce validated state in download and upload endpoints
AlexRubik May 19, 2026
d69fab5
feat(dirt): add ReadLatest SP and skip empty ReportData in V1 SP
AlexRubik May 19, 2026
c50e572
feat(dirt): dispatch GetLatest endpoint by V2 feature flag
AlexRubik May 19, 2026
8561e00
docs(dirt): document JSON substring coupling on EF ReadLatest filter
AlexRubik May 20, 2026
d5c0bd9
style(dirt): reformat OrganizationReport SPs per SQL keyword style
AlexRubik May 21, 2026
dcd4ec0
Update src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_GetLates…
mkincaid-bw May 21, 2026
d3e0cf9
Update src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_ReadLate…
mkincaid-bw May 21, 2026
c38911a
Update util/Migrator/DbScripts/2026-05-19_00_AddOrganizationReportRea…
mkincaid-bw May 21, 2026
06adae0
Update util/Migrator/DbScripts/2026-05-19_01_UpdateOrganizationReport…
mkincaid-bw May 21, 2026
64cdaa6
test(dirt): align V2 happy-path test name with FlagOn convention
AlexRubik Jun 2, 2026
9abee6f
fix(dirt): widen V2 read SP to include V1 inline fallback
AlexRubik Jun 2, 2026
67a4b12
fix(dirt): restore Validated=true pattern match in V2 controller branch
AlexRubik Jun 3, 2026
addf175
test(dirt): dispose MemoryStream in two DownloadReportFileAsync tests
AlexRubik Jun 3, 2026
c7e0353
Merge branch 'main' into dirt/pm-34909/serve-validated-report-files
AlexRubik Jun 3, 2026
8186ab8
chore(dirt): rename DbUp migrations to merge date 2026-06-03
AlexRubik Jun 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions src/Api/Dirt/Controllers/OrganizationReportsController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,16 +141,15 @@ public async Task<IActionResult> GetLatestOrganizationReportAsync(Guid organizat

await AuthorizeAsync(organizationId);

var latestReport = await _getOrganizationReportQuery.GetLatestOrganizationReportAsync(organizationId);
var isAccessIntelligenceV2 = _featureService.IsEnabled(FeatureFlagKeys.AccessIntelligenceVersion2);

if (latestReport == null)
{
throw new NotFoundException();
}
Comment thread
Banrion marked this conversation as resolved.
var latestReport = isAccessIntelligenceV2
? await _getOrganizationReportQuery.ReadLatestOrganizationReportAsync(organizationId)
: await _getOrganizationReportQuery.GetLatestOrganizationReportAsync(organizationId);

var response = new OrganizationReportResponseModel(latestReport);

if (_featureService.IsEnabled(FeatureFlagKeys.AccessIntelligenceVersion2))
if (isAccessIntelligenceV2)
{
var fileData = latestReport.GetReportFile();
if (fileData is { Validated: true })
Expand Down Expand Up @@ -393,7 +392,7 @@ public async Task UploadReportFileAsync(Guid organizationId, Guid reportId, [Fro
}

var fileData = report.GetReportFile();
if (fileData == null || fileData.Id != reportFileId)
if (fileData == null || fileData.Id != reportFileId || fileData.Validated)
{
throw new NotFoundException();
}
Expand Down Expand Up @@ -449,6 +448,11 @@ public async Task<IActionResult> DownloadReportFileAsync(Guid organizationId, Gu
throw new NotFoundException();
}

if (_featureService.IsEnabled(FeatureFlagKeys.AccessIntelligenceVersion2) && !fileData.Validated)
Comment thread
Banrion marked this conversation as resolved.
{
throw new NotFoundException();
}

var stream = await _storageService.GetReportReadStreamAsync(report, fileData);
if (stream == null)
{
Expand Down
15 changes: 15 additions & 0 deletions src/Core/Dirt/Reports/ReportFeatures/GetOrganizationReportQuery.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,21 @@ public async Task<OrganizationReport> GetLatestOrganizationReportAsync(Guid orga
return result;
}

public async Task<OrganizationReport> ReadLatestOrganizationReportAsync(Guid organizationId)
{
_logger.LogInformation(Constants.BypassFiltersEventId,
"Reading latest validated-file organization report for organization {organizationId}",
organizationId);
var result = await _organizationReportRepo.ReadLatestByOrganizationIdAsync(organizationId);

if (result == null)
{
throw new NotFoundException($"No validated report found for organization: {organizationId}");
}

return result;
}

public async Task<OrganizationReport> GetOrganizationReportAsync(Guid reportId)
{
_logger.LogInformation(Constants.BypassFiltersEventId, "Fetching organization reports for organization by Id: {reportId}", reportId);
Expand Down
1 change: 1 addition & 0 deletions src/Core/Dirt/Reports/ReportFeatures/Interfaces/IGetOrganizationReportQuery.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ public interface IGetOrganizationReportQuery
{
Task<OrganizationReport> GetOrganizationReportAsync(Guid organizationId);
Task<OrganizationReport> GetLatestOrganizationReportAsync(Guid organizationId);
Task<OrganizationReport> ReadLatestOrganizationReportAsync(Guid organizationId);
}
1 change: 1 addition & 0 deletions src/Core/Dirt/Repositories/IOrganizationReportRepository.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ public interface IOrganizationReportRepository : IRepository<OrganizationReport,
{
// Whole OrganizationReport methods
Task<OrganizationReport> GetLatestByOrganizationIdAsync(Guid organizationId);
Task<OrganizationReport> ReadLatestByOrganizationIdAsync(Guid organizationId);

// SummaryData methods
Task<IEnumerable<OrganizationReportSummaryDataResponse>> GetSummaryDataByDateRangeAsync(Guid organizationId, DateTime startDate, DateTime endDate);
Expand Down
13 changes: 13 additions & 0 deletions src/Infrastructure.Dapper/Dirt/OrganizationReportRepository.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,19 @@ public async Task<OrganizationReport> GetLatestByOrganizationIdAsync(Guid organi
}
}

public async Task<OrganizationReport> ReadLatestByOrganizationIdAsync(Guid organizationId)
{
using (var connection = new SqlConnection(ReadOnlyConnectionString))
{
var result = await connection.QuerySingleOrDefaultAsync<OrganizationReport>(
$"[{Schema}].[OrganizationReport_ReadLatestByOrganizationId]",
new { OrganizationId = organizationId },
commandType: CommandType.StoredProcedure);

return result;
}
}

public async Task<OrganizationReport> UpdateSummaryDataAsync(Guid organizationId, Guid reportId, string summaryData)
{
using (var connection = new SqlConnection(ConnectionString))
Expand Down
29 changes: 28 additions & 1 deletion src/Infrastructure.EntityFramework/Dirt/Repositories/OrganizationReportRepository.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,34 @@ public async Task<OrganizationReport> GetLatestByOrganizationIdAsync(Guid organi
{
var dbContext = GetDatabaseContext(scope);
var result = await dbContext.OrganizationReports
.Where(p => p.OrganizationId == organizationId)
.Where(p => p.OrganizationId == organizationId
&& p.ReportData != string.Empty)
.OrderByDescending(p => p.RevisionDate)
.Take(1)
.FirstOrDefaultAsync();

if (result == null) return default;

return Mapper.Map<OrganizationReport>(result);
}
}

public async Task<OrganizationReport> ReadLatestByOrganizationIdAsync(Guid organizationId)
{
using (var scope = ServiceScopeFactory.CreateScope())
{
var dbContext = GetDatabaseContext(scope);
// Substring match relies on SetReportFile (OrganizationReport.cs) serializing via
// JsonHelpers.IgnoreWritingNull (PascalCase, no whitespace). If those serializer
// options ever change, the substring stops matching V2 rows on non-MSSQL providers
// and the OR fallback silently serves the latest V1 inline row instead, masking
// the bug. The Dapper/MSSQL path uses JSON_VALUE which is format-agnostic.
var result = await dbContext.OrganizationReports
.Where(p => p.OrganizationId == organizationId
&& (
(p.ReportFile != null && p.ReportFile.Contains("\"Validated\":true"))
|| p.ReportData != string.Empty
))
.OrderByDescending(p => p.RevisionDate)
.Take(1)
.FirstOrDefaultAsync();
Expand Down
10 changes: 7 additions & 3 deletions src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_GetLatestByOrganizationId.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ BEGIN

SELECT TOP 1
*
FROM [dbo].[OrganizationReportView]
WHERE [OrganizationId] = @OrganizationId
ORDER BY [RevisionDate] DESC
FROM
[dbo].[OrganizationReportView]
WHERE
[OrganizationId] = @OrganizationId
AND [ReportData] <> ''
ORDER BY
[RevisionDate] DESC
END
19 changes: 19 additions & 0 deletions src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_ReadLatestByOrganizationId.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
CREATE PROCEDURE [dbo].[OrganizationReport_ReadLatestByOrganizationId]
Copy link
Copy Markdown
Contributor

@mkincaid-bw mkincaid-bw May 20, 2026

Choose a reason for hiding this comment

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

Since OrganizationReport_GetLatestByOrganizationId should have been named OrganizationReport_ReadLatestByOrganizationId, the naming of this new procedure insinuates these two procs return the same data. If this new proc will eventually replace OrganizationReport_GetLatestByOrganizationId, then I'm okay with it; otherwise I think it should be named something else to differentiate that it returns different data, maybe OrganizationReport_ReadLatestValidatedByOrganizationId?

Copy link
Copy Markdown
Contributor Author

@AlexRubik AlexRubik May 21, 2026

Choose a reason for hiding this comment

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

If this new proc will eventually replace OrganizationReport_GetLatestByOrganizationId, then I'm okay with it

Yes that's the plan, once pm-31920-access-intelligence-azure-file-storage flag is permanently ON, the V1 OrganizationReport_GetLatestByOrganizationId SP becomes dead code and we remove it, leaving OrganizationReport_ReadLatestByOrganizationId as the only latest-by-org SP.

@OrganizationId UNIQUEIDENTIFIER
AS
BEGIN
SET NOCOUNT ON

SELECT TOP 1
*
FROM
[dbo].[OrganizationReportView]
WHERE
[OrganizationId] = @OrganizationId
AND (
JSON_VALUE([ReportFile], '$.Validated') = 'true'
OR [ReportData] <> ''
)
ORDER BY
[RevisionDate] DESC
END
Loading
Loading