fix(security): keep imported mail subjects out of the admin audit
This commit is contained in:
@@ -159,6 +159,82 @@ public sealed class AdminProcessingControllerTests
|
||||
Assert.Equal(expected, (await client.GetAsync(route)).StatusCode);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Audit_log_keeps_mail_subjects_in_the_owner_record()
|
||||
{
|
||||
await using var host = await Host.Start();
|
||||
const string privateSubject = "Synthetic private correspondence subject";
|
||||
const string operationalNote = "Synthetic workflow status change";
|
||||
await using (var scope = host.Services.CreateAsyncScope())
|
||||
{
|
||||
var db = scope.ServiceProvider.GetRequiredService<JobTrackerContext>();
|
||||
db.JobEvents.AddRange(
|
||||
new JobEvent { JobApplicationId = 2, Type = "ReplyReceived", NewValue = "True:2026-11-03", Note = privateSubject, At = DateTime.UtcNow },
|
||||
new JobEvent { JobApplicationId = 2, Type = "StatusChanged", OldValue = "Applied", NewValue = "Interview", Note = operationalNote, At = DateTime.UtcNow });
|
||||
await db.SaveChangesAsync();
|
||||
}
|
||||
|
||||
using var anonymous = host.Client(null);
|
||||
using var ordinary = host.Client("owner");
|
||||
foreach (var (client, status) in new[] { (anonymous, HttpStatusCode.Unauthorized), (ordinary, HttpStatusCode.Forbidden) })
|
||||
{
|
||||
using var response = await client.GetAsync("/api/admin/audit");
|
||||
Assert.Equal(status, response.StatusCode);
|
||||
Assert.DoesNotContain(privateSubject, await response.Content.ReadAsStringAsync());
|
||||
}
|
||||
|
||||
using var admin = host.Client("admin");
|
||||
var audit = await admin.GetFromJsonAsync<JsonElement>("/api/admin/audit");
|
||||
var events = audit.EnumerateArray().ToDictionary(item => item.GetProperty("type").GetString()!);
|
||||
Assert.DoesNotContain(privateSubject, audit.GetRawText());
|
||||
var reply = events["ReplyReceived"];
|
||||
Assert.Equal(JsonValueKind.Null, reply.GetProperty("note").ValueKind);
|
||||
Assert.Equal("True:2026-11-03", reply.GetProperty("newValue").GetString());
|
||||
Assert.Equal(2, reply.GetProperty("jobApplicationId").GetInt32());
|
||||
Assert.Equal(operationalNote, events["StatusChanged"].GetProperty("note").GetString());
|
||||
|
||||
await using var verifyScope = host.Services.CreateAsyncScope();
|
||||
var verifyDb = verifyScope.ServiceProvider.GetRequiredService<JobTrackerContext>();
|
||||
Assert.Equal(privateSubject, await verifyDb.JobEvents.IgnoreQueryFilters().Where(item => item.Type == "ReplyReceived").Select(item => item.Note).SingleAsync());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task User_polling_does_not_expose_another_owners_notifications_or_processing_records()
|
||||
{
|
||||
await using var host = await Host.Start();
|
||||
var foreignNoticeId = Guid.NewGuid();
|
||||
const string foreignNotice = "Synthetic notice for the other owner";
|
||||
await using (var scope = host.Services.CreateAsyncScope())
|
||||
{
|
||||
var db = scope.ServiceProvider.GetRequiredService<JobTrackerContext>();
|
||||
db.UserNotifications.AddRange(
|
||||
new UserNotification { Id = Guid.NewGuid(), OwnerUserId = "owner", Kind = "operation_failed", Title = "Operation failed", Message = "Synthetic owner notice", CreatedAtUtc = DateTime.UtcNow },
|
||||
new UserNotification { Id = foreignNoticeId, OwnerUserId = "other", Kind = "operation_failed", Title = "Operation failed", Message = foreignNotice, CreatedAtUtc = DateTime.UtcNow });
|
||||
await db.SaveChangesAsync();
|
||||
}
|
||||
|
||||
using var owner = host.Client("owner");
|
||||
var notices = await owner.GetFromJsonAsync<JsonElement>("/api/notifications?owner=other");
|
||||
Assert.Equal("Synthetic owner notice", Assert.Single(notices.EnumerateArray()).GetProperty("message").GetString());
|
||||
var unread = await owner.GetFromJsonAsync<JsonElement>("/api/notifications/unread-count?owner=other");
|
||||
Assert.Equal(1, unread.GetProperty("count").GetInt32());
|
||||
Assert.Equal(HttpStatusCode.NotFound, (await owner.PostAsync($"/api/notifications/{foreignNoticeId}/read", null)).StatusCode);
|
||||
Assert.Equal(HttpStatusCode.NotFound, (await owner.DeleteAsync($"/api/notifications/{foreignNoticeId}")).StatusCode);
|
||||
Assert.Equal(HttpStatusCode.NotFound, (await owner.GetAsync($"/api/operations/{host.ForeignSubjectOperationId}")).StatusCode);
|
||||
var ownOperation = await owner.GetStringAsync($"/api/operations/{host.OperationId}");
|
||||
foreach (var internalValue in new[] { "SECRET_TOKEN", "SECRET_EXCEPTION", "SECRET_RESULT", "SECRET_CV_CONTENT" })
|
||||
Assert.DoesNotContain(internalValue, ownOperation);
|
||||
|
||||
using var other = host.Client("other");
|
||||
notices = await other.GetFromJsonAsync<JsonElement>("/api/notifications");
|
||||
Assert.Equal(foreignNotice, Assert.Single(notices.EnumerateArray()).GetProperty("message").GetString());
|
||||
await using var verifyScope = host.Services.CreateAsyncScope();
|
||||
var verifyDb = verifyScope.ServiceProvider.GetRequiredService<JobTrackerContext>();
|
||||
var notice = await verifyDb.UserNotifications.IgnoreQueryFilters().SingleAsync(item => item.Id == foreignNoticeId);
|
||||
Assert.Null(notice.ReadAtUtc);
|
||||
Assert.Null(notice.DismissedAtUtc);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Admin_projection_filters_owners_and_never_returns_raw_processing_contents_or_lease_data()
|
||||
{
|
||||
@@ -216,7 +292,9 @@ public sealed class AdminProcessingControllerTests
|
||||
var builder = WebApplication.CreateBuilder();
|
||||
builder.Logging.ClearProviders(); builder.WebHost.UseUrls("http://127.0.0.1:0");
|
||||
builder.Services.AddControllers().AddApplicationPart(typeof(AuthController).Assembly);
|
||||
builder.Services.AddAuthentication("test").AddScheme<AuthenticationSchemeOptions, TestAuthentication>("test", _ => { });
|
||||
builder.Services.AddAuthentication("test")
|
||||
.AddScheme<AuthenticationSchemeOptions, TestAuthentication>("test", _ => { })
|
||||
.AddScheme<AuthenticationSchemeOptions, TestAuthentication>("local", _ => { });
|
||||
builder.Services.AddAuthorization(); builder.Services.AddHttpContextAccessor();
|
||||
builder.Services.AddRateLimiter(options =>
|
||||
{
|
||||
@@ -226,6 +304,8 @@ public sealed class AdminProcessingControllerTests
|
||||
_ => new FixedWindowRateLimiterOptions { PermitLimit = permits, Window = TimeSpan.FromMinutes(1), QueueLimit = 0 }));
|
||||
});
|
||||
builder.Services.AddScoped<ICurrentUserService, CurrentUserService>();
|
||||
builder.Services.AddScoped<UserOperationStore>();
|
||||
builder.Services.AddScoped<UserNotificationStore>();
|
||||
builder.Services.AddSingleton(TimeProvider.System);
|
||||
builder.Services.AddDbContext<JobTrackerContext>(options => options.UseSqlite($"Data Source={Path.Combine(directory, "processing.db")};Pooling=False"));
|
||||
var app = builder.Build();
|
||||
|
||||
@@ -51,7 +51,8 @@ public sealed class AdminAuditController : ControllerBase
|
||||
e.Type,
|
||||
e.OldValue,
|
||||
e.NewValue,
|
||||
e.Note,
|
||||
// Imported replies keep the mailbox subject in the owner's timeline.
|
||||
Note = e.Type == "ReplyReceived" ? null : e.Note,
|
||||
e.At,
|
||||
e.JobApplicationId
|
||||
})
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
# Admin information isolation, offline increment
|
||||
|
||||
9 November 2026. Base `de3d5031c8208bc3841a528ae100ec2984d68501` on `codex/security-offline-preflight`. Compared the audit, Gmail, operations and fixture owners with A3 checkpoint `91e5a6bf`; their files were identical before this patch. Testing uses ephemeral loopback Kestrel and SQLite with synthetic owners and a test authentication scheme. No real mailbox, model, credential, provider, staging host or production service was accessed.
|
||||
|
||||
## Confirmed, privileged audit projection
|
||||
|
||||
The Gmail import creates a `ReplyReceived` event with the original mail subject in `JobEvent.Note`. The Admin-only Audit endpoint reads events across owners and previously sent that subject to administrators in the generic note field. A synthetic reply proved the subject was returned to an authenticated administrator. Anonymous and ordinary users were denied in the same test. This is excessive visibility to an existing administrative role, with a mailbox-import precondition. The fixture did not show an ordinary-user role bypass.
|
||||
|
||||
The read projection now omits `Note` for `ReplyReceived`. This also covers historical events without touching stored rows. The event type, response metadata, application association and ordinary status notes remain available. The owner's original event is preserved. Roles, undo/restore, notification producers and persisted data are unchanged.
|
||||
|
||||
## Covered boundaries
|
||||
|
||||
| Boundary | Evidence |
|
||||
| --- | --- |
|
||||
| Admin Audit | Loopback HTTP verifies 401/403 and the redacted Admin response. The allowed event metadata and a non-private status note remain. Stored owner subject still matches the synthetic input. |
|
||||
| Admin Processing | Existing HTTP tests verify all three endpoints deny ordinary/anonymous callers, keep role and bounded rate enforcement, and omit raw CV content, file paths and lease data from allowed diagnostics. |
|
||||
| User notifications | HTTP list/count ignore a forged owner query. A foreign notification ID returns 404 for mark-read and dismiss. The other owner still sees their own notice. Relational state confirms neither mutation occurred. |
|
||||
| User operations | The owner cannot retrieve the other owner's operation by ID. The owner's own detail omits raw failure text, lease token, result reference and CV content. Public task stage/category remain part of the existing contract. |
|
||||
| Pollers and producers | Reviewed EF owner filters and operation, saved-search and interrupted-mail producers. These write owner-bound user messages. No admin event producer was identified in the examined notification store. System and Users server role requirements were inspected and covered by existing authorization posture tests. |
|
||||
| Public liveness/readiness | Source shows the existing minimal liveness version/status and readiness status contracts. Neither substitutes for the role-gated system diagnostic payload. No live checks run. |
|
||||
|
||||
The red test failed once on the original HTTP response. The final four selected suites passed 62 cases, with zero failures/skips and exit 0. Pinned .NET 9.0.317, locked restore from the local cache and Release tests with `--no-restore`. Dependency advisory checks were not run during the offline restore.
|
||||
|
||||
- [Red TRX](evidence/admin-information-isolation/red.trx.gz), uncompressed SHA-256 `3795be6331eb68ff9fef3648d1fea3d938f0333db7c755984eb5705c09f1cb56`.
|
||||
- [Green TRX](evidence/admin-information-isolation/green.trx.gz), uncompressed SHA-256 `a46e1c00dbf0e486495bce7a11524fe5b75d9975788e35b63f8e82c074eaeaf6`.
|
||||
|
||||
## Open
|
||||
|
||||
Broader audit minimisation, administrative actions, email and realtime delivery boundaries, actual MariaDB and integrated role/session/deployment checks remain open. The Codex Security plugin is present. Formal baseline, diff/deep, integrated and staging scan activities were **NOT RUN** in this independent offline increment. Source and fixture results do not satisfy those later gates. No support-access or recovery permissions changed. [MFA recovery proposal](MFA_RECOVERY_PROPOSAL.md)
|
||||
Binary file not shown.
Binary file not shown.
Reference in New Issue
Block a user