cesnimda/jobtrackingapp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(dashboard): protect schema snapshots during delayed writes

Browse Source
This commit is contained in:
cesnimda
2026-10-21 21:05:40 +02:00
parent 5edbe59692
commit a24e6b3159
3 changed files with 64 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
JobTrackerApi.Tests/DashboardPreferencesTests.cs
+54 -3
View File
@@ -1,6 +1,7 @@
using System.Net;
using System.Net.Http.Json;
using System.Security.Claims;
using System.Data.Common;
using System.Text;
using System.Text.Encodings.Web;
using System.Text.Json;
@@ -14,6 +15,7 @@ using Microsoft.AspNetCore.Hosting.Server.Features;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Migrations;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.AspNetCore.Identity;
using Microsoft.Extensions.Options;
using Moq;
@@ -141,6 +143,31 @@ public sealed class DashboardPreferencesTests
Assert.Equal(4, stored.DashboardLayoutRevision);
}
[Fact]
public async Task Future_schema_installed_while_body_is_paused_cannot_be_overwritten_with_guessed_revision()
{
await using var host = await Host.Start();
using var client = host.Client("owner");
var preRead = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
var releaseBody = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
host.ReadObserver.OnLayoutRead = () => preRead.TrySetResult();
using var body = new PausedJson("{\"version\":1,\"revision\":1,\"widgets\":[]}", releaseBody.Task);
var pending = client.PutAsync(Route, body);
try
{
await preRead.Task.WaitAsync(TimeSpan.FromSeconds(10));
await using var db = host.Db();
const string future = "{\"version\":2,\"widgets\":[{\"id\":\"future-widget\",\"custom\":\"preserve\"}]}";
await db.Database.ExecuteSqlInterpolatedAsync($"UPDATE AspNetUsers SET DashboardLayoutJson={future}, DashboardLayoutRevision=1 WHERE Id='owner'");
releaseBody.TrySetResult();
Assert.Equal(HttpStatusCode.Conflict, (await pending).StatusCode);
var stored = await db.Users.AsNoTracking().Where(x => x.Id == "owner").Select(x => new { x.DashboardLayoutJson, x.DashboardLayoutRevision }).SingleAsync();
Assert.Equal(future, stored.DashboardLayoutJson);
Assert.Equal(1, stored.DashboardLayoutRevision);
}
finally { releaseBody.TrySetResult(); }
}
[Fact]
public async Task All_known_widgets_and_empty_reset_save_roundtrip_in_order()
{
@@ -216,12 +243,35 @@ public sealed class DashboardPreferencesTests
protected override bool TryComputeLength(out long length) { length = 0; return false; }
}
private sealed class PausedJson(string value, Task release) : HttpContent
{
protected override async Task SerializeToStreamAsync(Stream stream, TransportContext? context)
{
await stream.WriteAsync(new byte[] { (byte)' ' });
await stream.FlushAsync();
await release;
await stream.WriteAsync(Encoding.UTF8.GetBytes(value));
}
protected override bool TryComputeLength(out long length) { length = 0; return false; }
}
private sealed class LayoutReadObserver : DbCommandInterceptor
{
public Action? OnLayoutRead { get; set; }
public override InterceptionResult DataReaderDisposing(DbCommand command, DataReaderDisposingEventData eventData, InterceptionResult result)
{
if (command.CommandText.StartsWith("SELECT", StringComparison.Ordinal) && command.CommandText.Contains("DashboardLayoutJson", StringComparison.Ordinal)) OnLayoutRead?.Invoke();
return result;
}
}
private sealed class Host : IAsyncDisposable
{
private readonly WebApplication app;
private readonly string directory;
private readonly string connection;
private Host(WebApplication app, string directory, string connection) => (this.app, this.directory, this.connection) = (app, directory, connection);
public LayoutReadObserver ReadObserver { get; }
private Host(WebApplication app, string directory, string connection, LayoutReadObserver readObserver) => (this.app, this.directory, this.connection, ReadObserver) = (app, directory, connection, readObserver);
public JobTrackerContext Db() => new(new DbContextOptionsBuilder<JobTrackerContext>().UseSqlite(connection).Options, Mock.Of<ICurrentUserService>());
public HttpClient Client(string? user)
{
@@ -241,7 +291,8 @@ public sealed class DashboardPreferencesTests
builder.Services.AddControllers().AddApplicationPart(typeof(AuthController).Assembly);
builder.Services.AddAuthentication("local").AddScheme<AuthenticationSchemeOptions, TestAuthentication>("local", _ => { });
builder.Services.AddAuthorization();
builder.Services.AddDbContext<JobTrackerContext>(x => x.UseSqlite(connection));
var readObserver = new LayoutReadObserver();
builder.Services.AddDbContext<JobTrackerContext>(x => x.UseSqlite(connection).AddInterceptors(readObserver));
builder.Services.AddSingleton(Mock.Of<ICurrentUserService>());
builder.Services.AddSingleton<AppPaths>();
builder.Services.AddSingleton<IStartupReadiness, StartupReadiness>();
@@ -255,7 +306,7 @@ public sealed class DashboardPreferencesTests
app.UseAuthentication();
app.UseAuthorization();
app.MapControllers();
var host = new Host(app, directory, connection);
var host = new Host(app, directory, connection, readObserver);
await using (var db = host.Db())
{
if (legacy)
JobTrackerApi/Controllers/AuthController.cs
+3 -1
View File
@@ -562,7 +562,7 @@ public sealed class AuthController : ControllerBase
var userId = LocalAuthIdentity.GetRequiredUserId(User);
if (userId is null) return Unauthorized();
var existing = await _db.Users.AsNoTracking().Where(x => x.Id == userId)
.Select(x => new { x.DashboardLayoutJson }).SingleOrDefaultAsync(cancellationToken);
.Select(x => new { x.DashboardLayoutJson, x.DashboardLayoutRevision }).SingleOrDefaultAsync(cancellationToken);
if (existing is null) return Unauthorized();
if (DashboardLayout.Read(existing.DashboardLayoutJson, 0).Version > 1)
return Conflict(new { error = "dashboard_version_unsupported" });
@@ -585,6 +585,8 @@ public sealed class AuthController : ControllerBase
}
catch (JsonException) { return BadRequest("Invalid dashboard layout JSON."); }
if (layout is null) return BadRequest("Invalid dashboard layout version, revision or widgets.");
if (layout.Revision != existing.DashboardLayoutRevision)
return Conflict(new { error = "dashboard_revision_conflict" });
var json = JsonSerializer.Serialize(layout, new JsonSerializerOptions(JsonSerializerDefaults.Web));
var updated = await _db.Users.Where(x => x.Id == userId && x.DashboardLayoutRevision == layout.Revision)
.ExecuteUpdateAsync(set => set.SetProperty(x => x.DashboardLayoutJson, json)
docs/redesign/phase-8/p8-03/BACKEND.md
+7 -2
View File
@@ -12,6 +12,8 @@ Reads retain the database revision. Unknown, duplicate or malformed saved widget
`ApplicationUser` owns nullable `DashboardLayoutJson` and default-zero `DashboardLayoutRevision`. The endpoint uses one SQL `ExecuteUpdateAsync` guarded by owner ID and expected revision; it updates exactly these two columns. Simultaneous first writes cannot both win. No layout write invokes `UserManager.UpdateAsync`.
The future-schema guard and expected revision come from the same initial row snapshot. After the bounded body is read, its supplied revision must match that snapshot before the conditional SQL update runs. This closes a reproduced race where a paused chunked request could guess a newer revision installed with a future schema after the earlier schema check. The deterministic HTTP regression observes EF's completed initial row read, installs version 2/revision 1 on another connection while the body is held, then releases a version-1/revision-1 body. Before repair it returned 200 and overwrote the future settings; after repair it must return 409 and preserve the exact JSON/revision. The existing SQL revision predicate also rejects an intervening write after the snapshot comparison.
A reproduced additional race showed that an unrelated stale Identity whole-user update could otherwise replay the old layout/revision. Both layout properties ignore ordinary EF tracked updates after insertion. Their conditional endpoint update remains effective. The regression marks every property modified, matching Identity's store behaviour, then proves the newer layout survives. Profile, language, notification settings and Identity stamps remain unchanged by layout saves.
## Migration and startup
@@ -35,6 +37,9 @@ All commands used the pinned executable `C:\Users\Connor\.dotnet\dotnet.exe` thr
| Complete backend | no filter | **1081 pass / 0 fail / 0 skipped**, 29 seconds | `backend-full/dashboard-full.trx` |
| Future-schema / JS-safe revision RED | `FullyQualifiedName~Future_schema\|FullyQualifiedName~Invalid_payload_does_not_save` | 12 pass / 2 fail; future version hidden and unsafe exhausted revision accepted | `backend-future-red/dashboard-future-red.trx` |
| Complete backend after future-schema repair | no filter | **1083 pass / 0 fail / 0 skipped**, 32 seconds | `backend-full-final/dashboard-full.trx` |
| Future-schema paused-body race RED | `FullyQualifiedName~Future_schema_installed` | 0 pass / 1 fail; returned 200 instead of 409 | `backend-toctou-red/dashboard-toctou-red.trx` |
| Paused-body repair focused | `FullyQualifiedName~DashboardPreferencesTests` | **24 pass / 0 fail / 0 skipped**, 24 seconds | `backend-toctou-focused/dashboard-toctou-focused.trx` |
| Complete backend after paused-body repair | no filter | **1084 pass / 0 fail / 0 skipped**, 33 seconds | `backend-toctou-full/dashboard-full.trx` |
The initial test-harness attempt omitted historical startup reconciliation and failed before reaching endpoints (missing historical AvatarImageDataUrl column). The meaningful RED above reran after wiring the actual startup owner. The subsequent malformed fixture SQL used a literal brace through EF's formatting API; parameterising it corrected the fixture without changing the product expectation.
@@ -47,10 +52,10 @@ rtk C:\Users\Connor\.dotnet\dotnet.exe test JobTrackerApi.Tests/JobTrackerApi.Te
Final complete command:
```powershell
rtk C:\Users\Connor\.dotnet\dotnet.exe test JobTrackerApi.Tests/JobTrackerApi.Tests.csproj --configuration Release --no-restore --logger "trx;LogFileName=dashboard-full.trx" --results-directory .artifacts/phase8/backend-full-final
rtk C:\Users\Connor\.dotnet\dotnet.exe test JobTrackerApi.Tests/JobTrackerApi.Tests.csproj --configuration Release --no-restore --logger "trx;LogFileName=dashboard-full.trx" --results-directory .artifacts/phase8/backend-toctou-full
```
The 23 new dashboard cases use an actual local Kestrel HTTP listener, real EF SQLite files, the complete migrations and actual startup reconciliation. They verify 401, owner isolation despite an attempted query account override, GET/PUT acknowledgement, 409, concurrent first saves, unchanged unrelated account fields/stamps, stale Identity preservation, invalid/oversized/chunked payloads, all catalogue IDs/order, empty saved lists, corrupt reads, future-schema downgrade protection, exact JavaScript revision bounds, legacy defaults and provider SQL generation. They also hold a SELECT reader open while saving the actual layout against the WAL file.
The 24 new dashboard cases use an actual local Kestrel HTTP listener, real EF SQLite files, the complete migrations and actual startup reconciliation. They verify 401, owner isolation despite an attempted query account override, GET/PUT acknowledgement, 409, concurrent first saves, unchanged unrelated account fields/stamps, stale Identity preservation, invalid/oversized/chunked payloads, all catalogue IDs/order, empty saved lists, corrupt reads, future-schema downgrade protection including a deterministic paused-body race, exact JavaScript revision bounds, legacy defaults and provider SQL generation. They also hold a SELECT reader open while saving the actual layout against the WAL file.
## Limits