Merge security/hardening-phase4: Phase 4-7 security hardening + CI
CI / backend (push) Successful in 1m4s
CI / frontend (push) Successful in 20s

Phase 1-5 security remediation (SSRF egress guard, systemic IDOR global query
filters, cookie Secure, forwarded-header trust, error-leak/ProblemDetails, pagination
clamps, security headers/HSTS, open-redirect, info-leak, non-root container, Postgres
lockdown), the Critical destructive-action UX safety fix, and a Gitea CI pipeline.
Re-validated safe to merge; 39 tests green. Deferred: CSRF token, key encryption,
CSP enforce, AI-egress governance. Ops follow-up: rotate Google OAuth secret, set
strong POSTGRES_PASSWORD.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
cesnimda
2026-07-01 00:33:20 +02:00
22 changed files with 679 additions and 54 deletions
+38
View File
@@ -0,0 +1,38 @@
name: CI
# Build + test gate. Runs on pushes to the long-lived branches and on every PR so
# the 39-test suite (and both builds) must pass before merge.
on:
push:
branches: [main, develop]
pull_request:
jobs:
backend:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-dotnet@v4
with:
dotnet-version: '8.0.x'
- name: Restore
run: dotnet restore InboxIntel.sln
- name: Build
run: dotnet build InboxIntel.sln -c Release --no-restore
- name: Test
run: dotnet test InboxIntel.sln -c Release --no-build --verbosity normal
frontend:
runs-on: ubuntu-latest
defaults:
run:
working-directory: frontend
steps:
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
node-version: '20'
- name: Install
run: npm ci
- name: Build
run: npm run build
+68
View File
@@ -0,0 +1,68 @@
\# Project Rules
\## Code Quality
\- Prefer readability over cleverness.
\- Keep methods under \~40 lines where practical.
\- Avoid duplicated logic.
\- Follow SOLID principles.
\- Keep files focused on a single responsibility.
\## Safety
\- Never commit secrets.
\- Never disable tests to make them pass.
\- Never remove functionality without explaining why.
\## Testing
Every change must include:
\- Unit tests where appropriate.
\- Integration tests for API changes.
\- Build verification.
\## Architecture
Prefer:
\- Dependency Injection
\- Composition over inheritance
\- Async APIs
\- Immutable models where practical
\## Documentation
Update documentation whenever public behaviour changes.
+12 -4
View File
@@ -4,11 +4,16 @@ services:
environment: environment:
POSTGRES_DB: inboxintel POSTGRES_DB: inboxintel
POSTGRES_USER: inboxintel POSTGRES_USER: inboxintel
POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-inboxintel} # V-03: require an explicit strong password (fail fast if POSTGRES_PASSWORD is unset)
# rather than silently defaulting to a guessable one.
POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:?set POSTGRES_PASSWORD in deploy/.env}
volumes: volumes:
- pgdata:/var/lib/postgresql/data - pgdata:/var/lib/postgresql/data
# V-03: bind to loopback only so the database is reachable from the host for local
# tooling but NOT from other machines on the network. The api container reaches it
# over the internal compose network regardless of this published port.
ports: ports:
- "5432:5432" - "127.0.0.1:5432:5432"
healthcheck: healthcheck:
test: ["CMD-SHELL", "pg_isready -U inboxintel"] test: ["CMD-SHELL", "pg_isready -U inboxintel"]
interval: 5s interval: 5s
@@ -22,7 +27,7 @@ services:
environment: environment:
ASPNETCORE_ENVIRONMENT: Production ASPNETCORE_ENVIRONMENT: Production
ASPNETCORE_URLS: http://+:8080 ASPNETCORE_URLS: http://+:8080
ConnectionStrings__Postgres: "Host=postgres;Port=5432;Database=inboxintel;Username=inboxintel;Password=${POSTGRES_PASSWORD:-inboxintel}" ConnectionStrings__Postgres: "Host=postgres;Port=5432;Database=inboxintel;Username=inboxintel;Password=${POSTGRES_PASSWORD:?set POSTGRES_PASSWORD in deploy/.env}"
DataProtection__KeyPath: /keys DataProtection__KeyPath: /keys
GoogleOAuth__ClientId: ${GOOGLE_CLIENT_ID:-} GoogleOAuth__ClientId: ${GOOGLE_CLIENT_ID:-}
GoogleOAuth__ClientSecret: ${GOOGLE_CLIENT_SECRET:-} GoogleOAuth__ClientSecret: ${GOOGLE_CLIENT_SECRET:-}
@@ -37,8 +42,11 @@ services:
depends_on: depends_on:
postgres: postgres:
condition: service_healthy condition: service_healthy
# V-08: bind to loopback so the API is not directly reachable from the network
# (only via the frontend/nginx proxy over the internal compose network). This
# prevents external clients from bypassing the proxy to spoof X-Forwarded-* headers.
ports: ports:
- "8080:8080" - "127.0.0.1:8080:8080"
frontend: frontend:
build: build:
+6
View File
@@ -68,6 +68,12 @@ Defined once in `src/index.css`; Tailwind theme references them so `bg-backgroun
} }
``` ```
> **Accent is swappable by design.** The accent lives in exactly one token (`--primary`,
> plus its dark variant). Changing the brand color = editing those two lines. This also
> makes a **future user-facing accent picker** cheap: store a chosen hue on the user (or
> in `localStorage`) and write `--primary`/`--ring` at runtime. Decision (2026-06-30):
> ship with **Indigo `#5b5bf0`**; leave the picker as a documented future enhancement.
### Proposed palette (for sign-off) ### Proposed palette (for sign-off)
| Token | Light | Dark | Use | | Token | Light | Dark | Use |
+81 -18
View File
@@ -1,30 +1,93 @@
import { useState } from 'react';
import { MailOpen, Mail, Star, Archive, Trash2 } from 'lucide-react';
import { BulkApi } from '../api/client.js'; import { BulkApi } from '../api/client.js';
import {
Button, useToast,
Dialog, DialogContent, DialogHeader, DialogTitle, DialogDescription, DialogFooter, DialogClose,
} from './ui';
/// <summary> /**
/// Toolbar shown above an email list when one or more rows are selected. * Toolbar shown above an email list when rows are selected.
/// `onDone` is called with the action key after a successful bulk call so the *
/// caller can update local state (e.g. remove trashed/archived rows). * Safety (Phase 4 / UX-Critical): the destructive Trash action now requires an
/// </summary> * explicit confirmation dialog, every action surfaces success/partial-failure via
* a toast, and rows are only removed from the list when the server confirms the
* whole batch succeeded (no more optimistic removal that hides failures).
* `onDone(action, ids)` is called only on full success so the caller can prune state.
*/
export default function BulkToolbar({ selectedIds, onDone, onClear }) { export default function BulkToolbar({ selectedIds, onDone, onClear }) {
const { toast } = useToast();
const [busy, setBusy] = useState(false);
const [confirmTrash, setConfirmTrash] = useState(false);
const count = selectedIds.length; const count = selectedIds.length;
if (count === 0) return null; if (count === 0) return null;
const run = (fn, action) => async () => { const apply = async (fn, action, label) => {
await fn(selectedIds); setBusy(true);
onDone(action, selectedIds); try {
onClear(); const res = await fn(selectedIds);
// CleanupResultDto: { succeededCount, failedCount, errors }
const ok = res?.succeededCount ?? count;
const failed = res?.failedCount ?? 0;
if (failed > 0) {
toast({
variant: 'warning',
title: `${label}: ${ok} done, ${failed} failed`,
description: 'Some items could not be updated — the list was left unchanged so you can retry.',
});
} else {
toast({ variant: 'success', title: `${label} ${ok} email${ok === 1 ? '' : 's'}` });
onDone(action, selectedIds);
onClear();
}
} catch {
toast({ variant: 'danger', title: `Couldn't ${label.toLowerCase()} ${count} email${count === 1 ? '' : 's'}`, description: 'Please try again.' });
} finally {
setBusy(false);
setConfirmTrash(false);
}
}; };
return ( return (
<div className="bulk-toolbar"> <div className="flex flex-wrap items-center gap-2 rounded-lg border border-border bg-card px-3 py-2 shadow-sm">
<span>{count} selected</span> <span className="text-sm font-medium">{count} selected</span>
<div className="bulk-toolbar-spacer" /> <div className="flex-1" />
<button className="bulk-btn" onClick={run(BulkApi.markRead, 'read')}>Mark read</button> <Button variant="ghost" size="sm" disabled={busy} onClick={() => apply(BulkApi.markRead, 'read', 'Marked read')}>
<button className="bulk-btn" onClick={run(BulkApi.markUnread, 'unread')}>Mark unread</button> <MailOpen /> Read
<button className="bulk-btn" onClick={run(BulkApi.star, 'star')}>Star</button> </Button>
<button className="bulk-btn" onClick={run(BulkApi.archive, 'archive')}>Archive</button> <Button variant="ghost" size="sm" disabled={busy} onClick={() => apply(BulkApi.markUnread, 'unread', 'Marked unread')}>
<button className="bulk-btn bulk-btn--danger" onClick={run(BulkApi.trash, 'trash')}>Trash</button> <Mail /> Unread
<button className="bulk-btn" onClick={onClear}>Cancel</button> </Button>
<Button variant="ghost" size="sm" disabled={busy} onClick={() => apply(BulkApi.star, 'star', 'Starred')}>
<Star /> Star
</Button>
<Button variant="ghost" size="sm" disabled={busy} onClick={() => apply(BulkApi.archive, 'archive', 'Archived')}>
<Archive /> Archive
</Button>
<Button variant="danger-outline" size="sm" disabled={busy} onClick={() => setConfirmTrash(true)}>
<Trash2 /> Trash
</Button>
<Button variant="ghost" size="sm" disabled={busy} onClick={onClear}>Cancel</Button>
<Dialog open={confirmTrash} onOpenChange={(o) => !busy && setConfirmTrash(o)}>
<DialogContent>
<DialogHeader>
<DialogTitle>Move {count} email{count === 1 ? '' : 's'} to Trash?</DialogTitle>
<DialogDescription>
This moves the selected mail to your Gmail Trash, where it stays recoverable
for 30 days before Gmail permanently deletes it.
</DialogDescription>
</DialogHeader>
<DialogFooter>
<DialogClose asChild>
<Button variant="outline" size="sm" disabled={busy}>Cancel</Button>
</DialogClose>
<Button variant="danger" size="sm" disabled={busy} onClick={() => apply(BulkApi.trash, 'trash', 'Trashed')}>
{busy ? 'Moving…' : 'Move to Trash'}
</Button>
</DialogFooter>
</DialogContent>
</Dialog>
</div> </div>
); );
} }
@@ -16,11 +16,13 @@ public class AnalyticsController : ApiControllerBase
[HttpGet("top-senders")] [HttpGet("top-senders")]
public async Task<IActionResult> TopSenders([FromQuery] int take = 20, CancellationToken ct = default) public async Task<IActionResult> TopSenders([FromQuery] int take = 20, CancellationToken ct = default)
=> Ok(await _analytics.GetTopSendersAsync(UserId, take, ct)); // V-10: clamp to a sane bound (the SPA legitimately requests up to 5000 to list
// all senders) so a caller cannot force an unbounded scan.
=> Ok(await _analytics.GetTopSendersAsync(UserId, Math.Clamp(take, 1, 5000), ct));
[HttpGet("volume")] [HttpGet("volume")]
public async Task<IActionResult> Volume([FromQuery] int days = 90, CancellationToken ct = default) public async Task<IActionResult> Volume([FromQuery] int days = 90, CancellationToken ct = default)
=> Ok(await _analytics.GetVolumeOverTimeAsync(UserId, days, ct)); => Ok(await _analytics.GetVolumeOverTimeAsync(UserId, Math.Clamp(days, 1, 3660), ct));
[HttpGet("heatmap")] [HttpGet("heatmap")]
public async Task<IActionResult> Heatmap(CancellationToken ct) => Ok(await _analytics.GetHeatmapAsync(UserId, ct)); public async Task<IActionResult> Heatmap(CancellationToken ct) => Ok(await _analytics.GetHeatmapAsync(UserId, ct));
@@ -28,10 +28,17 @@ public class AppInfoController : ControllerBase
public IActionResult Info() public IActionResult Info()
{ {
var devMode = _config.GetValue<bool?>("App:DevMode") ?? _env.IsDevelopment(); var devMode = _config.GetValue<bool?>("App:DevMode") ?? _env.IsDevelopment();
// V-13: when NOT in dev mode, disclose nothing beyond the flag to anonymous
// callers. The dev banner (the only consumer of environment/maxMessages) only
// renders when devMode is true, so this preserves the feature without leaking
// the environment name or sync cap in production.
if (!devMode)
return Ok(new { devMode = false });
return Ok(new return Ok(new
{ {
devMode = true,
environment = _env.EnvironmentName, environment = _env.EnvironmentName,
devMode,
maxMessages = _config.GetValue<int>("GmailSync:MaxMessages") maxMessages = _config.GetValue<int>("GmailSync:MaxMessages")
}); });
} }
@@ -17,7 +17,12 @@ public class AuthController : ControllerBase
[HttpGet("login")] [HttpGet("login")]
[AllowAnonymous] [AllowAnonymous]
public IActionResult Login([FromQuery] string? returnUrl = "/") public IActionResult Login([FromQuery] string? returnUrl = "/")
=> Challenge(new AuthenticationProperties { RedirectUri = returnUrl }, GoogleDefaults.AuthenticationScheme); {
// V-09: only allow local post-login redirects; reject absolute/off-host targets
// so the OAuth flow can't be abused as an open redirect for phishing.
var safe = !string.IsNullOrEmpty(returnUrl) && Url.IsLocalUrl(returnUrl) ? returnUrl : "/app";
return Challenge(new AuthenticationProperties { RedirectUri = safe }, GoogleDefaults.AuthenticationScheme);
}
[HttpPost("logout")] [HttpPost("logout")]
[Authorize] [Authorize]
@@ -7,19 +7,31 @@ namespace InboxIntel.Api.Controllers;
public class SearchController : ApiControllerBase public class SearchController : ApiControllerBase
{ {
/// <summary>Hard upper bound on a user-requested page (V-10: DoS via huge pageSize).
/// Enforced HERE, at the user-facing trust boundary, so internal callers of
/// ISearchService (e.g. cleanup target resolution) can still request large pages.</summary>
private const int MaxPageSize = 200;
private readonly ISearchService _search; private readonly ISearchService _search;
public SearchController(ISearchService search) => _search = search; public SearchController(ISearchService search) => _search = search;
/// <summary>Structured search via JSON body.</summary> /// <summary>Structured search via JSON body.</summary>
[HttpPost] [HttpPost]
public async Task<IActionResult> Search([FromBody] SearchRequestDto request, CancellationToken ct) public async Task<IActionResult> Search([FromBody] SearchRequestDto request, CancellationToken ct)
=> Ok(await _search.SearchAsync(UserId, request, ct)); {
var clamped = request with
{
Page = Math.Max(1, request.Page),
PageSize = Math.Clamp(request.PageSize, 1, MaxPageSize)
};
return Ok(await _search.SearchAsync(UserId, clamped, ct));
}
/// <summary>Gmail-like query string search, e.g. ?q=from:github.com is:unread.</summary> /// <summary>Gmail-like query string search, e.g. ?q=from:github.com is:unread.</summary>
[HttpGet] [HttpGet]
public async Task<IActionResult> Query([FromQuery] string? q, [FromQuery] int page = 1, [FromQuery] int pageSize = 50, CancellationToken ct = default) public async Task<IActionResult> Query([FromQuery] string? q, [FromQuery] int page = 1, [FromQuery] int pageSize = 50, CancellationToken ct = default)
{ {
var parsed = GmailQueryParser.Parse(q, page, pageSize); var parsed = GmailQueryParser.Parse(q, Math.Max(1, page), Math.Clamp(pageSize, 1, MaxPageSize));
return Ok(await _search.SearchAsync(UserId, parsed, ct)); return Ok(await _search.SearchAsync(UserId, parsed, ct));
} }
} }
+8
View File
@@ -16,5 +16,13 @@ RUN dotnet publish src/InboxIntel.Api/InboxIntel.Api.csproj -c Release -o /app/p
FROM mcr.microsoft.com/dotnet/aspnet:8.0 AS runtime FROM mcr.microsoft.com/dotnet/aspnet:8.0 AS runtime
WORKDIR /app WORKDIR /app
COPY --from=build /app/publish . COPY --from=build /app/publish .
# V-12: run as the non-root 'app' user shipped in the .NET 8 images. Pre-create the
# DataProtection key directory owned by that user so the (initially empty) 'keys'
# volume inherits app ownership on first mount and key persistence still works.
# NOTE: an EXISTING root-owned keys volume must be recreated for this to take effect.
RUN mkdir -p /keys && chown -R app:app /keys /app
USER app
EXPOSE 8080 EXPOSE 8080
ENTRYPOINT ["dotnet", "InboxIntel.Api.dll"] ENTRYPOINT ["dotnet", "InboxIntel.Api.dll"]
+50 -9
View File
@@ -48,6 +48,12 @@ builder.Services.AddAuthentication(options =>
{ {
options.Cookie.HttpOnly = true; options.Cookie.HttpOnly = true;
options.Cookie.SameSite = SameSiteMode.Lax; options.Cookie.SameSite = SameSiteMode.Lax;
// V-04: never emit the session cookie over plain HTTP in non-dev. Behind nginx
// the forwarded proto (now only trusted from known proxies, see below) makes
// Always work; local http://localhost dev still functions via SameAsRequest.
options.Cookie.SecurePolicy = builder.Environment.IsDevelopment()
? CookieSecurePolicy.SameAsRequest
: CookieSecurePolicy.Always;
options.Cookie.Name = "inboxintel.session"; options.Cookie.Name = "inboxintel.session";
options.ExpireTimeSpan = TimeSpan.FromDays(7); options.ExpireTimeSpan = TimeSpan.FromDays(7);
options.SlidingExpiration = true; options.SlidingExpiration = true;
@@ -94,6 +100,9 @@ builder.Services.AddApiVersioning(o =>
builder.Services.AddControllers(); builder.Services.AddControllers();
builder.Services.AddEndpointsApiExplorer(); builder.Services.AddEndpointsApiExplorer();
builder.Services.AddSwaggerGen(); builder.Services.AddSwaggerGen();
// V-06: RFC7807 ProblemDetails so the global exception handler returns a safe,
// generic error body instead of leaking framework stack traces / internal messages.
builder.Services.AddProblemDetails();
builder.Services.AddCors(o => o.AddPolicy("frontend", p => p builder.Services.AddCors(o => o.AddPolicy("frontend", p => p
.WithOrigins(builder.Configuration.GetSection("Cors:Origins").Get<string[]>() ?? new[] { "http://localhost:5173" }) .WithOrigins(builder.Configuration.GetSection("Cors:Origins").Get<string[]>() ?? new[] { "http://localhost:5173" })
@@ -109,21 +118,53 @@ using (var scope = app.Services.CreateScope())
await db.Database.MigrateAsync(); await db.Database.MigrateAsync();
} }
// Honor X-Forwarded-* from the nginx reverse proxy so OAuth redirect URIs and
// cookie Secure flags reflect the external scheme/host, not the container's.
// V-08: only trust these headers from KNOWN proxy networks (configurable). The
// default covers private/Docker ranges so the compose nginx works, while a client
// reaching the API directly can no longer spoof scheme/host/forwarded-for.
var forwardedOptions = new ForwardedHeadersOptions
{
ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto | ForwardedHeaders.XForwardedHost,
ForwardLimit = app.Configuration.GetValue<int?>("ForwardedHeaders:ForwardLimit") ?? 1
};
forwardedOptions.KnownNetworks.Clear();
forwardedOptions.KnownProxies.Clear();
var trustedNetworks = app.Configuration.GetSection("ForwardedHeaders:KnownNetworks").Get<string[]>()
?? new[] { "10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16", "127.0.0.0/8", "::1/128" };
foreach (var cidr in trustedNetworks)
{
var parts = cidr.Split('/');
if (parts.Length == 2 && System.Net.IPAddress.TryParse(parts[0], out var prefix) && int.TryParse(parts[1], out var len))
forwardedOptions.KnownNetworks.Add(new Microsoft.AspNetCore.HttpOverrides.IPNetwork(prefix, len));
}
app.UseForwardedHeaders(forwardedOptions);
if (app.Environment.IsDevelopment()) if (app.Environment.IsDevelopment())
{ {
app.UseSwagger(); app.UseSwagger();
app.UseSwaggerUI(); app.UseSwaggerUI();
} }
else
// Honor X-Forwarded-* from the nginx reverse proxy so OAuth redirect URIs and
// cookie Secure flags reflect the external scheme/host, not the container's.
var forwardedOptions = new ForwardedHeadersOptions
{ {
ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto | ForwardedHeaders.XForwardedHost // V-06: generic ProblemDetails for unhandled exceptions (no stack traces to clients).
}; app.UseExceptionHandler();
forwardedOptions.KnownNetworks.Clear(); // V-11: HSTS once TLS is enforced at the proxy (forwarded proto now trustworthy).
forwardedOptions.KnownProxies.Clear(); app.UseHsts();
app.UseForwardedHeaders(forwardedOptions); }
// V-11: baseline security response headers. CSP is report-only for now so it can be
// tuned against the SPA before enforcing (the SPA itself is also served with headers
// by its nginx). Applied to API responses here as defense-in-depth.
app.Use(async (ctx, next) =>
{
var h = ctx.Response.Headers;
h["X-Content-Type-Options"] = "nosniff";
h["X-Frame-Options"] = "DENY";
h["Referrer-Policy"] = "no-referrer";
h["Cross-Origin-Opener-Policy"] = "same-origin";
await next();
});
app.UseSerilogRequestLogging(); app.UseSerilogRequestLogging();
app.UseCors("frontend"); app.UseCors("frontend");
+4
View File
@@ -51,6 +51,10 @@
"FrequencyDays": 7, "FrequencyDays": 7,
"SendHourUtc": 8 "SendHourUtc": 8
}, },
"ForwardedHeaders": {
"ForwardLimit": 1,
"KnownNetworks": [ "10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16", "127.0.0.0/8", "::1/128" ]
},
"Cors": { "Cors": {
"Origins": [ "http://localhost:5173" ] "Origins": [ "http://localhost:5173" ]
}, },
@@ -100,8 +100,9 @@ public class CleanupService : ICleanupService
catch (Exception ex) catch (Exception ex)
{ {
_logger.LogError(ex, "Cleanup action {Action} failed for user {UserId}", request.Action, userId); _logger.LogError(ex, "Cleanup action {Action} failed for user {UserId}", request.Action, userId);
errors.Add(ex.Message); // V-06: do not echo internal exception detail to the client.
return Result<CleanupResultDto>.Failure(ex.Message); errors.Add("The cleanup action could not be completed.");
return Result<CleanupResultDto>.Failure("The cleanup action could not be completed.");
} }
} }
@@ -5,6 +5,7 @@ using InboxIntel.Domain.Entities;
using InboxIntel.Domain.Enums; using InboxIntel.Domain.Enums;
using InboxIntel.Infrastructure.Gmail; using InboxIntel.Infrastructure.Gmail;
using InboxIntel.Infrastructure.Persistence; using InboxIntel.Infrastructure.Persistence;
using InboxIntel.Infrastructure.Security;
using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging;
@@ -120,13 +121,17 @@ public class UnsubscribeService : IUnsubscribeService
switch (item.Method) switch (item.Method)
{ {
case UnsubscribeMethod.OneClickPost: case UnsubscribeMethod.OneClickPost:
var post = await http.PostAsync(item.UnsubscribeTarget, // V-01: the target comes from an attacker-authored email header.
// Validate against SSRF (scheme + private/metadata ranges) before fetching.
var postUri = await SafeHttpGuard.ValidateAsync(item.UnsubscribeTarget, ct);
var post = await http.PostAsync(postUri,
new StringContent("List-Unsubscribe=One-Click"), ct); new StringContent("List-Unsubscribe=One-Click"), ct);
SetResult(item, post.IsSuccessStatusCode); SetResult(item, post.IsSuccessStatusCode);
if (post.IsSuccessStatusCode) ok++; else fail++; if (post.IsSuccessStatusCode) ok++; else fail++;
break; break;
case UnsubscribeMethod.HttpLink: case UnsubscribeMethod.HttpLink:
var get = await http.GetAsync(item.UnsubscribeTarget, ct); var getUri = await SafeHttpGuard.ValidateAsync(item.UnsubscribeTarget, ct);
var get = await http.GetAsync(getUri, ct);
SetResult(item, get.IsSuccessStatusCode); SetResult(item, get.IsSuccessStatusCode);
if (get.IsSuccessStatusCode) ok++; else fail++; if (get.IsSuccessStatusCode) ok++; else fail++;
break; break;
@@ -140,12 +145,24 @@ public class UnsubscribeService : IUnsubscribeService
break; break;
} }
} }
catch (Exception ex) catch (SsrfBlockedException ex)
{ {
// Blocked target (e.g. points at an internal/metadata address). Log
// server-side; surface only a generic reason to the client.
fail++; fail++;
item.Status = UnsubscribeStatus.Failed; item.Status = UnsubscribeStatus.Failed;
item.ResultMessage = ex.Message; item.ResultMessage = "Unsubscribe link was blocked for safety.";
errors.Add($"{item.UnsubscribeTarget}: {ex.Message}"); _logger.LogWarning(ex, "Blocked unsubscribe target for user {UserId}", userId);
errors.Add("An unsubscribe link was blocked for safety.");
}
catch (Exception ex)
{
// V-06: log detail, return a generic message.
fail++;
item.Status = UnsubscribeStatus.Failed;
item.ResultMessage = "Unsubscribe request failed.";
_logger.LogWarning(ex, "Unsubscribe request failed for user {UserId}", userId);
errors.Add("An unsubscribe request failed.");
} }
} }
await _db.SaveChangesAsync(ct); await _db.SaveChangesAsync(ct);
@@ -60,7 +60,10 @@ public static class DependencyInjection
services.AddHostedService<DigestWorker>(); services.AddHostedService<DigestWorker>();
// HTTP clients // HTTP clients
services.AddHttpClient("unsubscribe", c => c.Timeout = TimeSpan.FromSeconds(15)); // V-01: do NOT follow redirects — a validated external URL must not be able to
// 3xx-redirect into an internal target after SafeHttpGuard has checked it.
services.AddHttpClient("unsubscribe", c => c.Timeout = TimeSpan.FromSeconds(15))
.ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler { AllowAutoRedirect = false });
services.AddHttpClient("ollama"); services.AddHttpClient("ollama");
services.AddHttpClient("openai"); services.AddHttpClient("openai");
@@ -8,7 +8,22 @@ namespace InboxIntel.Infrastructure.Persistence;
public class AppDbContext : DbContext, IAppDbContext public class AppDbContext : DbContext, IAppDbContext
{ {
public AppDbContext(DbContextOptions<AppDbContext> options) : base(options) { } private readonly ICurrentUser? _currentUser;
public AppDbContext(DbContextOptions<AppDbContext> options, ICurrentUser? currentUser = null) : base(options)
=> _currentUser = currentUser;
/// <summary>
/// Tenant id used by the global query filters below. Resolves to the
/// authenticated user during an HTTP request. It is <see cref="Guid.Empty"/>
/// when there is no current user (background workers, design-time tooling,
/// startup migration) — in which case filtering is DISABLED, because those
/// paths are trusted server code that already scope their own queries by a
/// userId passed in explicitly. The security value is on the HTTP attack
/// surface: a forgotten manual <c>WHERE UserId ==</c> can no longer leak
/// another tenant's rows, since the filter restricts to the caller.
/// </summary>
public Guid CurrentUserId => _currentUser?.UserId ?? Guid.Empty;
public DbSet<User> Users => Set<User>(); public DbSet<User> Users => Set<User>();
public DbSet<Email> Emails => Set<Email>(); public DbSet<Email> Emails => Set<Email>();
@@ -26,6 +41,41 @@ public class AppDbContext : DbContext, IAppDbContext
protected override void OnModelCreating(ModelBuilder modelBuilder) protected override void OnModelCreating(ModelBuilder modelBuilder)
{ {
modelBuilder.ApplyConfigurationsFromAssembly(Assembly.GetExecutingAssembly()); modelBuilder.ApplyConfigurationsFromAssembly(Assembly.GetExecutingAssembly());
// Defense-in-depth tenant isolation (systemic IDOR safeguard). Every
// user-owned entity is filtered to the current user so a query that forgets
// its manual `WHERE UserId ==` clause cannot leak across tenants. Applied
// uniformly to all user-scoped entities so EF sees no filtered/unfiltered
// navigation mismatch. Bypassed when CurrentUserId is Guid.Empty (workers).
modelBuilder.Entity<Email>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
modelBuilder.Entity<Sender>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
modelBuilder.Entity<MailThread>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
modelBuilder.Entity<MailDomain>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
modelBuilder.Entity<Attachment>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
modelBuilder.Entity<Label>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
modelBuilder.Entity<SyncState>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
modelBuilder.Entity<AnalyticsAggregate>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
modelBuilder.Entity<WidgetLayout>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
modelBuilder.Entity<UnsubscribeItem>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
// PostgreSQL full-text search: generated tsvector over subject + body with a
// GIN index, maintained by the DB and read-only in code. The tsvector type is
// Npgsql-only, so map it only for relational providers and ignore it otherwise
// (e.g. the InMemory provider used by tests). Production behaviour is unchanged.
if (Database.IsRelational())
{
modelBuilder.Entity<Email>().Property(e => e.SearchVector)
.HasColumnType("tsvector")
.HasComputedColumnSql(
"to_tsvector('english', coalesce(\"Subject\",'') || ' ' || coalesce(\"BodyText\",''))",
stored: true);
modelBuilder.Entity<Email>().HasIndex(e => e.SearchVector).HasMethod("GIN");
}
else
{
modelBuilder.Entity<Email>().Ignore(e => e.SearchVector);
}
base.OnModelCreating(modelBuilder); base.OnModelCreating(modelBuilder);
} }
@@ -36,13 +36,8 @@ public class EmailConfiguration : IEntityTypeConfiguration<Email>
.HasForeignKey(e => e.SenderId) .HasForeignKey(e => e.SenderId)
.OnDelete(DeleteBehavior.Restrict); .OnDelete(DeleteBehavior.Restrict);
// PostgreSQL full-text search: generated tsvector over subject + body, // NOTE: the PostgreSQL full-text `tsvector` mapping is applied in
// with a GIN index. Maintained by the database, read-only in code. // AppDbContext.OnModelCreating, guarded by Database.IsRelational(), so that
b.Property(e => e.SearchVector) // non-relational test providers (InMemory) can ignore the Npgsql-only type.
.HasColumnType("tsvector")
.HasComputedColumnSql(
"to_tsvector('english', coalesce(\"Subject\",'') || ' ' || coalesce(\"BodyText\",''))",
stored: true);
b.HasIndex(e => e.SearchVector).HasMethod("GIN");
} }
} }
@@ -19,6 +19,9 @@ public class SearchService : ISearchService
public async Task<PagedResult<EmailSummaryDto>> SearchAsync(Guid userId, SearchRequestDto r, CancellationToken ct = default) public async Task<PagedResult<EmailSummaryDto>> SearchAsync(Guid userId, SearchRequestDto r, CancellationToken ct = default)
{ {
// NOTE: pagination is clamped at the user-facing trust boundary (SearchController),
// NOT here, so internal callers (e.g. CleanupService target resolution, which
// legitimately requests large pages) are unaffected. See V-10 fix.
var q = _db.Emails.AsNoTracking().Where(e => e.UserId == userId); var q = _db.Emails.AsNoTracking().Where(e => e.UserId == userId);
if (!string.IsNullOrWhiteSpace(r.Sender)) if (!string.IsNullOrWhiteSpace(r.Sender))
@@ -0,0 +1,94 @@
using System.Net;
using System.Net.Sockets;
namespace InboxIntel.Infrastructure.Security;
/// <summary>
/// Validates outbound request URLs before the server fetches them, to prevent
/// SSRF (V-01). Used by the unsubscribe processor (URLs come from attacker-authored
/// List-Unsubscribe headers) and intended for any future server-initiated fetch
/// (AI/breach providers). Enforces an http/https scheme allowlist and rejects hosts
/// that resolve to loopback / private / link-local / unique-local / multicast ranges
/// or the cloud metadata address. DNS is resolved and EVERY resolved address is
/// checked, defeating DNS-rebinding to an external name that points at an internal IP.
///
/// Pair this with an HttpClient configured with AllowAutoRedirect = false so a
/// permitted external URL cannot 3xx-redirect into an internal target post-validation.
/// </summary>
public static class SafeHttpGuard
{
/// <summary>
/// Throws <see cref="SsrfBlockedException"/> if the URL is unsafe to fetch.
/// Returns the validated absolute Uri otherwise.
/// </summary>
public static async Task<Uri> ValidateAsync(string? url, CancellationToken ct = default)
{
if (string.IsNullOrWhiteSpace(url) || !Uri.TryCreate(url, UriKind.Absolute, out var uri))
throw new SsrfBlockedException("Unsubscribe target is not a valid absolute URL.");
if (uri.Scheme != Uri.UriSchemeHttp && uri.Scheme != Uri.UriSchemeHttps)
throw new SsrfBlockedException($"Disallowed URL scheme '{uri.Scheme}'.");
// Resolve the host; if it's already a literal IP, GetHostAddressesAsync returns it.
IPAddress[] addresses;
try
{
addresses = await Dns.GetHostAddressesAsync(uri.DnsSafeHost, ct);
}
catch (Exception ex) when (ex is SocketException or ArgumentException)
{
throw new SsrfBlockedException("Unsubscribe target host could not be resolved.");
}
if (addresses.Length == 0)
throw new SsrfBlockedException("Unsubscribe target host did not resolve to any address.");
foreach (var ip in addresses)
if (IsBlocked(ip))
throw new SsrfBlockedException($"Unsubscribe target resolves to a disallowed address ({ip}).");
return uri;
}
/// <summary>True if the address is in a range we must never fetch server-side.</summary>
public static bool IsBlocked(IPAddress ip)
{
if (IPAddress.IsLoopback(ip)) return true;
if (ip.AddressFamily == AddressFamily.InterNetwork)
{
var b = ip.GetAddressBytes(); // big-endian
// 0.0.0.0/8 (this host), 10/8, 100.64/10 (CGNAT), 127/8, 169.254/16 (link-local + metadata),
// 172.16/12, 192.0.0/24, 192.168/16, 255.255.255.255
if (b[0] == 0) return true;
if (b[0] == 10) return true;
if (b[0] == 100 && b[1] >= 64 && b[1] <= 127) return true;
if (b[0] == 127) return true;
if (b[0] == 169 && b[1] == 254) return true; // includes 169.254.169.254 metadata
if (b[0] == 172 && b[1] >= 16 && b[1] <= 31) return true;
if (b[0] == 192 && b[1] == 168) return true;
if (ip.Equals(IPAddress.Broadcast)) return true;
if (b[0] >= 224) return true; // multicast / reserved
return false;
}
if (ip.AddressFamily == AddressFamily.InterNetworkV6)
{
if (ip.IsIPv6LinkLocal || ip.IsIPv6SiteLocal || ip.IsIPv6Multicast) return true;
// IPv4-mapped (::ffff:a.b.c.d) — re-check the embedded v4 address.
if (ip.IsIPv4MappedToIPv6) return IsBlocked(ip.MapToIPv4());
var b = ip.GetAddressBytes();
// Unique-local fc00::/7
if ((b[0] & 0xFE) == 0xFC) return true;
return false;
}
return true; // unknown family — fail closed
}
}
/// <summary>Raised when an outbound URL is rejected by <see cref="SafeHttpGuard"/>.</summary>
public class SsrfBlockedException : Exception
{
public SsrfBlockedException(string message) : base(message) { }
}
@@ -0,0 +1,51 @@
using FluentAssertions;
using InboxIntel.Application.Abstractions;
using InboxIntel.Application.Search;
using InboxIntel.Domain.Entities;
using InboxIntel.Infrastructure.Persistence;
using InboxIntel.Infrastructure.Search;
using Microsoft.EntityFrameworkCore;
using Xunit;
namespace InboxIntel.IntegrationTests;
/// <summary>
/// Regression guard for the V-10 pagination clamp: the clamp must live at the
/// user-facing controller boundary, NOT in SearchService — otherwise internal
/// callers (CleanupService resolves targets with pageSize 10000) would be silently
/// capped, breaking bulk cleanup-by-query. This proves the service itself honours a
/// large page.
/// </summary>
public class SearchPaginationTests
{
private sealed class FakeCurrentUser : ICurrentUser
{
public Guid UserId { get; set; }
public bool IsAuthenticated => UserId != Guid.Empty;
}
[Fact]
public async Task SearchService_does_not_clamp_large_internal_page()
{
var user = Guid.NewGuid();
var opts = new DbContextOptionsBuilder<AppDbContext>()
.UseInMemoryDatabase(nameof(SearchService_does_not_clamp_large_internal_page)).Options;
using (var seed = new AppDbContext(opts, new FakeCurrentUser()))
{
var sender = new Sender { UserId = user, Address = "sender@example.com", DisplayName = "Sender" };
seed.Senders.Add(sender);
for (var i = 0; i < 250; i++)
seed.Emails.Add(new Email { UserId = user, GmailMessageId = $"m{i}", Subject = $"s{i}", SenderId = sender.Id, Sender = sender });
await seed.SaveChangesAsync();
}
using var ctx = new AppDbContext(opts, new FakeCurrentUser { UserId = user });
// Mirrors how CleanupService resolves targets: a large page via the parser.
var request = GmailQueryParser.Parse(null, page: 1, pageSize: 10_000);
var result = await new SearchService(ctx).SearchAsync(user, request);
result.Items.Should().HaveCount(250); // not capped at 200
result.TotalCount.Should().Be(250);
}
}
@@ -0,0 +1,78 @@
using FluentAssertions;
using InboxIntel.Application.Abstractions;
using InboxIntel.Domain.Entities;
using InboxIntel.Infrastructure.Persistence;
using Microsoft.EntityFrameworkCore;
using Xunit;
namespace InboxIntel.IntegrationTests;
/// <summary>
/// Verifies the global query-filter IDOR safeguard on AppDbContext: an HTTP-scoped
/// context (with a current user) sees only that user's rows even if a query forgets
/// its manual UserId filter; a worker-scoped context (Guid.Empty) sees everything.
/// </summary>
public class TenantIsolationTests
{
private sealed class FakeCurrentUser : ICurrentUser
{
public Guid UserId { get; set; }
public bool IsAuthenticated => UserId != Guid.Empty;
}
private static DbContextOptions<AppDbContext> InMemory(string name)
=> new DbContextOptionsBuilder<AppDbContext>().UseInMemoryDatabase(name).Options;
private static readonly Guid UserA = Guid.NewGuid();
private static readonly Guid UserB = Guid.NewGuid();
private static async Task SeedAsync(DbContextOptions<AppDbContext> opts)
{
// Seed with no current user (Guid.Empty) so the filter is bypassed for writes/reads here.
using var seed = new AppDbContext(opts, new FakeCurrentUser());
seed.Emails.Add(new Email { UserId = UserA, GmailMessageId = "a1", Subject = "A-one" });
seed.Emails.Add(new Email { UserId = UserA, GmailMessageId = "a2", Subject = "A-two" });
seed.Emails.Add(new Email { UserId = UserB, GmailMessageId = "b1", Subject = "B-one" });
await seed.SaveChangesAsync();
}
[Fact]
public async Task Authenticated_context_sees_only_its_own_rows_even_without_manual_filter()
{
var opts = InMemory(nameof(Authenticated_context_sees_only_its_own_rows_even_without_manual_filter));
await SeedAsync(opts);
// Note: NO manual .Where(e => e.UserId == ...) here — the global filter must enforce it.
using var ctx = new AppDbContext(opts, new FakeCurrentUser { UserId = UserA });
var emails = await ctx.Emails.ToListAsync();
emails.Should().HaveCount(2);
emails.Should().OnlyContain(e => e.UserId == UserA);
}
[Fact]
public async Task Other_users_row_is_invisible_by_id()
{
var opts = InMemory(nameof(Other_users_row_is_invisible_by_id));
await SeedAsync(opts);
using var ctx = new AppDbContext(opts, new FakeCurrentUser { UserId = UserA });
// Fetch B's row by its GmailMessageId — classic IDOR attempt; must return null.
var leaked = await ctx.Emails.FirstOrDefaultAsync(e => e.GmailMessageId == "b1");
leaked.Should().BeNull();
}
[Fact]
public async Task Empty_current_user_bypasses_the_filter_for_background_workers()
{
var opts = InMemory(nameof(Empty_current_user_bypasses_the_filter_for_background_workers));
await SeedAsync(opts);
// Guid.Empty == background/worker scope: must see all tenants' rows so sync/upsert works.
using var ctx = new AppDbContext(opts, new FakeCurrentUser { UserId = Guid.Empty });
var all = await ctx.Emails.ToListAsync();
all.Should().HaveCount(3);
}
}
@@ -0,0 +1,71 @@
using System.Net;
using FluentAssertions;
using InboxIntel.Infrastructure.Security;
using Xunit;
namespace InboxIntel.UnitTests;
/// <summary>
/// SSRF guard (V-01): outbound URLs derived from attacker-authored email headers
/// must not be allowed to target internal/metadata/private addresses or non-web schemes.
/// </summary>
public class SafeHttpGuardTests
{
[Theory]
[InlineData("169.254.169.254")] // cloud metadata
[InlineData("127.0.0.1")] // loopback
[InlineData("10.0.0.5")] // private A
[InlineData("172.16.4.4")] // private B
[InlineData("172.31.255.255")] // private B upper bound
[InlineData("192.168.1.1")] // private C
[InlineData("100.64.0.1")] // CGNAT
[InlineData("0.0.0.0")] // this-host
[InlineData("255.255.255.255")] // broadcast
[InlineData("224.0.0.1")] // multicast
public void Blocks_private_and_special_ipv4(string ip)
=> SafeHttpGuard.IsBlocked(IPAddress.Parse(ip)).Should().BeTrue();
[Theory]
[InlineData("::1")] // loopback
[InlineData("fe80::1")] // link-local
[InlineData("fc00::1")] // unique-local
[InlineData("fd12:3456::1")] // unique-local
[InlineData("::ffff:169.254.169.254")] // v4-mapped metadata
[InlineData("::ffff:10.0.0.1")] // v4-mapped private
public void Blocks_private_and_special_ipv6(string ip)
=> SafeHttpGuard.IsBlocked(IPAddress.Parse(ip)).Should().BeTrue();
[Theory]
[InlineData("8.8.8.8")]
[InlineData("93.184.216.34")] // example.com
[InlineData("2606:2800:220:1::1")]
public void Allows_public_addresses(string ip)
=> SafeHttpGuard.IsBlocked(IPAddress.Parse(ip)).Should().BeFalse();
[Theory]
[InlineData("ftp://example.com/x")]
[InlineData("file:///etc/passwd")]
[InlineData("gopher://example.com")]
[InlineData("not-a-url")]
[InlineData("")]
public async Task Rejects_non_http_schemes_and_garbage(string url)
{
var act = async () => await SafeHttpGuard.ValidateAsync(url);
await act.Should().ThrowAsync<SsrfBlockedException>();
}
[Fact]
public async Task Rejects_url_resolving_to_loopback()
{
// localhost resolves to a loopback address and must be blocked.
var act = async () => await SafeHttpGuard.ValidateAsync("http://localhost/unsub");
await act.Should().ThrowAsync<SsrfBlockedException>();
}
[Fact]
public async Task Rejects_literal_metadata_ip_url()
{
var act = async () => await SafeHttpGuard.ValidateAsync("http://169.254.169.254/latest/meta-data/");
await act.Should().ThrowAsync<SsrfBlockedException>();
}
}