diff --git a/docs/sprint23-progress.md b/docs/sprint23-progress.md new file mode 100644 index 0000000..547a0fe --- /dev/null +++ b/docs/sprint23-progress.md @@ -0,0 +1,26 @@ +# Sprint 23 — adversarial bug-hunt + +Цель: атаковать систему как злонамеренный пользователь. После 22 спринтов +система зрелая, но скрытые баги ТОЧНО есть. Найти как можно больше, +сразу починить. + +Старт: 2026-06-08. Исполнитель: Claude Opus 4.7. + +## Категории атак + +- [ ] **1. Race conditions** — параллельные POST на posting-endpoints +- [ ] **2. Auth edge** — JWT tampering / replay / SuperAdmin override +- [ ] **3. Multi-tenant via URL** — cross-org доступ через прямой ID +- [ ] **4. Validation bypass** — SQL/XSS/path-traversal/unicode +- [ ] **5. Numeric/decimal edge** — отрицательные, overflow, precision +- [ ] **6. Stock invariant** — 100 параллельных продаж +- [ ] **7. DOS protection** — SignalR flood, large bodies, slowloris +- [ ] **8. Hangfire-jobs safety** — admin-vs-superadmin, cross-tenant + +Каждый найденный баг → `reports/bugs-found-{n}.md` (github-style issue), +потом fix, потом retest. + +## Журнал + +### 2026-06-08 старт +Sprint 22 закрыт (7/7 ✓). Поехали по adversarial-attacks. diff --git a/src/food-market.api/Controllers/Catalog/ProductsController.cs b/src/food-market.api/Controllers/Catalog/ProductsController.cs index ffdafd9..ca0d42a 100644 --- a/src/food-market.api/Controllers/Catalog/ProductsController.cs +++ b/src/food-market.api/Controllers/Catalog/ProductsController.cs @@ -40,10 +40,17 @@ public ProductsController(AppDbContext db, ITenantContext tenant) .Select(pt => new { pt.Id, pt.Name }) .ToListAsync(ct); if (required.Count == 0) return null; + // Sprint 23 / bug-004: проверяем ОКРУГЛЁННОЕ значение, потому что + // после RoundIfNeeded(value, allowFractional) очень маленькая цена + // вроде 0.0000001 превращается в 0 — пользователь думал что прошла + // required-проверка (0.0000001 > 0), а в БД оказался ноль. + var allowFractional = await AllowFractionalAsync(ct); foreach (var pt in required) { var price = prices?.FirstOrDefault(p => p.PriceTypeId == pt.Id); - if (price is null || price.Amount <= 0m) return pt.Name; + if (price is null) return pt.Name; + var rounded = RoundIfNeeded(price.Amount, allowFractional); + if (rounded <= 0m) return pt.Name; } return null; } diff --git a/src/food-market.api/Controllers/Sales/RetailSalesController.cs b/src/food-market.api/Controllers/Sales/RetailSalesController.cs index 7810f6f..985f89c 100644 --- a/src/food-market.api/Controllers/Sales/RetailSalesController.cs +++ b/src/food-market.api/Controllers/Sales/RetailSalesController.cs @@ -549,7 +549,20 @@ public async Task Delete(Guid id, CancellationToken ct) } [HttpPost("{id:guid}/post"), RequiresPermission("RetailSalesOperate")] - public async Task Post(Guid id, CancellationToken ct) + public Task Post(Guid id, CancellationToken ct) => + // Sprint 23 / bug-003: Serializable transaction внутри PostCoreAsync + // может бросить 40001 при race. Раньше это поднималось до middleware + // и пользователь видел 500. Теперь SerializableRetry повторяет до 5 + // раз с exp backoff, на исчерпании возвращает 409 с понятным сообщением. + foodmarket.Api.Infrastructure.SerializableRetry.RunAsync( + ct, + async () => await PostCoreAsync(id, ct), + onExhausted: () => Conflict(new + { + error = "Слишком много одновременных операций с этими товарами. Подождите секунду и попробуйте ещё раз.", + })); + + private async Task PostCoreAsync(Guid id, CancellationToken ct) { var sale = await _db.RetailSales.Include(s => s.Lines).FirstOrDefaultAsync(s => s.Id == id, ct); if (sale is null) return NotFound(); diff --git a/src/food-market.api/Infrastructure/SerializableRetry.cs b/src/food-market.api/Infrastructure/SerializableRetry.cs new file mode 100644 index 0000000..228a63c --- /dev/null +++ b/src/food-market.api/Infrastructure/SerializableRetry.cs @@ -0,0 +1,73 @@ +using Microsoft.EntityFrameworkCore; +using Npgsql; + +namespace foodmarket.Api.Infrastructure; + +/// Sprint 23 / bug-003: helper для serialization-conflict retry'a. +/// +/// PostgreSQL Serializable isolation выбрасывает SqlState=40001 (serialization +/// failure) когда параллельная транзакция изменила данные. EF Core пропускает +/// исключение наверх, middleware возвращает 500. Это корректное поведение +/// БД, но плохой UX для пользователя. +/// +/// Этот helper выполняет блок до N раз с exp backoff + jitter; на исчерпании +/// retry-budget возвращает 409 Conflict через переданный onExhausted-делегат. +/// +/// Использование (в контроллере): +/// +/// return await SerializableRetry.RunAsync( +/// ct, +/// async () => { +/// await using var tx = await _db.Database.BeginTransactionAsync(IsolationLevel.Serializable, ct); +/// // … posting work … +/// await tx.CommitAsync(ct); +/// return NoContent(); +/// }, +/// onExhausted: () => Conflict(new { error = "..." }) +/// ); +/// +/// +public static class SerializableRetry +{ + /// Максимум попыток по умолчанию. + public const int DefaultMaxAttempts = 5; + + /// Запускает до раз, + /// перехватывая Postgres serialization failures (SqlState=40001). На + /// каждый retry — exp backoff + jitter (50ms × attempt + random 50ms). + /// На исчерпании — вызывает . + public static async Task RunAsync( + CancellationToken ct, + Func> action, + Func onExhausted, + int maxAttempts = DefaultMaxAttempts) + { + for (var attempt = 1; attempt <= maxAttempts; attempt++) + { + try + { + return await action(); + } + catch (Exception ex) when (IsSerializationFailure(ex)) + { + if (attempt >= maxAttempts) return onExhausted(); + var delay = TimeSpan.FromMilliseconds(50 * attempt + Random.Shared.Next(50)); + await Task.Delay(delay, ct); + } + } + // Достижимо только если maxAttempts=0 (некорректный конфиг). + return onExhausted(); + } + + /// Проверяет, является ли исключение Postgres serialization + /// failure (включая DbUpdateException-обёртки). + public static bool IsSerializationFailure(Exception ex) + { + if (ex is PostgresException pg && pg.SqlState == "40001") return true; + if (ex is DbUpdateException due && due.InnerException is PostgresException pg2 && pg2.SqlState == "40001") return true; + // EF Core может обернуть и в SqlException-альтернативу; раскрываем + // глубже на случай transient-обёрток. + if (ex.InnerException is not null) return IsSerializationFailure(ex.InnerException); + return false; + } +} diff --git a/src/food-market.api/Infrastructure/SerializationConflictMiddleware.cs b/src/food-market.api/Infrastructure/SerializationConflictMiddleware.cs new file mode 100644 index 0000000..3618303 --- /dev/null +++ b/src/food-market.api/Infrastructure/SerializationConflictMiddleware.cs @@ -0,0 +1,59 @@ +using Microsoft.EntityFrameworkCore; +using Npgsql; + +namespace foodmarket.Api.Infrastructure; + +/// Sprint 23 / bug-003: глобальный middleware-перехватчик PostgreSQL +/// serialization failure'ов (SqlState=40001). Без этого middleware параллельные +/// posting'и (RetailSale.Post, Supply.Post, Enter.Post, …) под Serializable +/// isolation возвращают 500 с пустым телом, что плохой UX и неинформативный +/// HTTP-код. +/// +/// Поведение: ловит exception, если это 40001 — возвращает 409 Conflict с +/// JSON `{ error: "...", retryable: true }`. UI должен retry'ить такие запросы +/// (или показать «попробуйте ещё раз»). +/// +/// Регистрировать ПЕРВЫМ в pipeline (до UseRouting) — чтобы поймать всё. +/// Не подменяет SerializableRetry-helper (тот retry'ит автоматически на +/// уровне действия; этот middleware — fallback для не-обёрнутых endpoint'ов). +public class SerializationConflictMiddleware +{ + private readonly RequestDelegate _next; + private readonly ILogger _log; + + public SerializationConflictMiddleware(RequestDelegate next, ILogger log) + { + _next = next; + _log = log; + } + + public async Task InvokeAsync(HttpContext ctx) + { + try + { + await _next(ctx); + } + catch (Exception ex) when (IsSerializationFailure(ex)) + { + _log.LogWarning("SerializationConflict: {Path} (retryable)", ctx.Request.Path); + if (ctx.Response.HasStarted) + { + // Уже отправили часть response — ничего не сделаем. + throw; + } + ctx.Response.Clear(); + ctx.Response.StatusCode = 409; + ctx.Response.ContentType = "application/json; charset=utf-8"; + await ctx.Response.WriteAsync( + "{\"error\":\"Конфликт параллельных операций. Подождите секунду и попробуйте ещё раз.\",\"retryable\":true}"); + } + } + + private static bool IsSerializationFailure(Exception ex) + { + if (ex is PostgresException pg && pg.SqlState == "40001") return true; + if (ex is DbUpdateException due && due.InnerException is PostgresException pg2 && pg2.SqlState == "40001") return true; + if (ex.InnerException is not null) return IsSerializationFailure(ex.InnerException); + return false; + } +} diff --git a/src/food-market.api/Infrastructure/Validation/Validators.cs b/src/food-market.api/Infrastructure/Validation/Validators.cs index 9b8718f..df68ace 100644 --- a/src/food-market.api/Infrastructure/Validation/Validators.cs +++ b/src/food-market.api/Infrastructure/Validation/Validators.cs @@ -2,6 +2,21 @@ namespace foodmarket.Api.Infrastructure.Validation; +/// Sprint 23 / bug-001: проверка что строка не содержит NUL-байт +/// и иных control-chars (кроме \t\r\n). Postgres TEXT/VARCHAR отказывается +/// принять \x00, что роняло SaveChanges с 500 без понятного ответа. +/// Применяется на пользовательские input-поля строкового типа. +internal static class ValidationExt +{ + public static bool NoControlChars(string? s) + { + if (s is null) return true; + foreach (var ch in s) + if (char.IsControl(ch) && ch != '\t' && ch != '\r' && ch != '\n') return false; + return true; + } +} + // ────────────────────────────────────────────────────────────────────────────── // FluentValidation-валидаторы для основных input-DTO. Зарегистрированы // автоматически через AddValidatorsFromAssemblyContaining(). @@ -66,9 +81,18 @@ public sealed class ProductInputValidator : AbstractValidator x.Name).NotEmpty().WithMessage("Название обязательно.") - .MaximumLength(200); - RuleFor(x => x.Article).MaximumLength(50); + .MaximumLength(500) + .Must(ValidationExt.NoControlChars) + .WithMessage("Поле Name не должно содержать управляющих символов (NUL и т.п.)."); + RuleFor(x => x.Article).MaximumLength(500) + .Must(s => ValidationExt.NoControlChars(s)).WithMessage("Поле Article не должно содержать управляющих символов."); + RuleFor(x => x.Description).Must(s => ValidationExt.NoControlChars(s)) + .WithMessage("Поле Description не должно содержать управляющих символов."); RuleFor(x => x.UnitOfMeasureId).NotEqual(Guid.Empty) .WithMessage("Не указана единица измерения."); RuleFor(x => x.Vat).InclusiveBetween(0m, 100m).When(x => x.Vat.HasValue) @@ -86,8 +110,15 @@ public sealed class CounterpartyInputValidator : AbstractValidator x.Name).NotEmpty().WithMessage("Название обязательно.") - .MaximumLength(200); + .MaximumLength(255) + .Must(ValidationExt.NoControlChars).WithMessage("Поле Name не должно содержать управляющих символов."); + RuleFor(x => x.LegalName).MaximumLength(500) + .Must(s => ValidationExt.NoControlChars(s)).WithMessage("Поле LegalName не должно содержать управляющих символов."); + RuleFor(x => x.Address).Must(s => ValidationExt.NoControlChars(s)).WithMessage("Поле Address не должно содержать управляющих символов."); + RuleFor(x => x.Notes).Must(s => ValidationExt.NoControlChars(s)).WithMessage("Поле Notes не должно содержать управляющих символов."); // БИН (юрлицо) — 12 цифр, ИИН (физлицо) — 12 цифр. Один из двух // обязателен только если тип LegalEntity (Type=1). Для Individual (2) // достаточно ИИН или имени. На уровне БД эти ограничения не enforced — diff --git a/src/food-market.api/Program.cs b/src/food-market.api/Program.cs index fc0e611..c2dfb0b 100644 --- a/src/food-market.api/Program.cs +++ b/src/food-market.api/Program.cs @@ -487,6 +487,11 @@ [new Microsoft.OpenApi.Models.OpenApiSecurityScheme // чтобы они применились даже на 429/403 от rate-limiter'а. app.UseMiddleware(); + // Sprint 23 / bug-003: глобальный перехватчик PostgreSQL serialization + // failure'ов (SqlState=40001) — мапит на 409 Conflict + retryable:true. + // Без него параллельные posting'и под Serializable отвечают 500. + app.UseMiddleware(); + app.UseSerilogRequestLogging(); app.UseCors(CorsPolicy); // Prometheus HTTP-метрики (http_requests_received_total, http_request_duration_seconds). diff --git a/tests/e2e/reports/bugs/bug-001-nullbyte-500.md b/tests/e2e/reports/bugs/bug-001-nullbyte-500.md new file mode 100644 index 0000000..6c63a0d Binary files /dev/null and b/tests/e2e/reports/bugs/bug-001-nullbyte-500.md differ diff --git a/tests/e2e/reports/bugs/bug-002-validator-length-mismatch.md b/tests/e2e/reports/bugs/bug-002-validator-length-mismatch.md new file mode 100644 index 0000000..9e53796 --- /dev/null +++ b/tests/e2e/reports/bugs/bug-002-validator-length-mismatch.md @@ -0,0 +1,42 @@ +# Bug #002 — ProductInput validator MaximumLength(200) vs StringLength(500) + +**Severity:** Low (UX, consistency) +**Component:** `Application/Catalog/CatalogDtos.cs` (ProductInput) + `Api/Infrastructure/Validation/Validators.cs` +**Found:** Sprint 23, 2026-06-08 + +## Воспроизведение + +- ProductInput.Name имеет `[StringLength(500)]` annotation. +- БД-колонка `products.Name` имеет `HasMaxLength(500)`. +- ProductInputValidator (FluentValidation) кидает 400 при Name > **200** символов. + +Пользователь читает аннотацию или DDL и ожидает 500 символов; на 201-чарном +имени получает ошибку с упоминанием 200 — расхождение. + +## Воспроизведение + +```bash +# 201 символов в name → 400 «The length of 'Name' must be 200 characters or fewer» +curl -X POST … -d '{"name":"Y...Y(500)","prices":[…]}' +``` + +## Ожидание + +Либо validator говорит 500 (совпадает с schema), либо schema режется до 200. +Sprint 23 фикс: validator → 500 (расширяем — чем меньше data loss, тем лучше). + +## Фикс + +```diff +- RuleFor(x => x.Name).NotEmpty().WithMessage("Название обязательно.") +- .MaximumLength(200); ++ RuleFor(x => x.Name).NotEmpty().WithMessage("Название обязательно.") ++ .MaximumLength(500); +``` + +То же для `CounterpartyInputValidator.Name` (200 → 255 — Counterparty.Name +имеет HasMaxLength(255)). + +## Retest + +После фикса: POST с 201/499 символов → 201. POST с 501 символом → 400. diff --git a/tests/e2e/reports/bugs/bug-003-serializable-500.md b/tests/e2e/reports/bugs/bug-003-serializable-500.md new file mode 100644 index 0000000..25aee55 --- /dev/null +++ b/tests/e2e/reports/bugs/bug-003-serializable-500.md @@ -0,0 +1,67 @@ +# Bug #003 — Serializable conflict → 500 (нужно 409 + retry) + +**Severity:** **CRITICAL** (visible incorrect HTTP-код на race; кассир видит «ошибка сервера» вместо корректного «попробуй ещё раз») +**Component:** `RetailSalesController.Post` (и любой posting endpoint, который BeginTransactionAsync(Serializable)) +**Found:** Sprint 23, 2026-06-08 + +## Воспроизведение + +20 параллельных POST `/api/sales/retail/{id}/post` на один продукт при stock=10: +``` +ok=8 conflict409=0 500=12 (12 пятисоток с empty body) +``` + +Stock-инвариант сохраняется (`stock=2.0 == Σ movements`), over-sell'a НЕТ ✓. +Но `500 + empty body` — это серверная ошибка, кассиру нечего сказать «попробуй +ещё раз», UI показывает «Ошибка сервера». + +## Причина + +В `RetailSalesController.Post`: +```csharp +await using var tx = await _db.Database.BeginTransactionAsync(IsolationLevel.Serializable, ct); +… +await tx.CommitAsync(ct); // ← здесь Postgres может бросить 40001 +``` + +PostgreSQL Serializable выбрасывает `SqlState=40001` при serialization failure +(другая транзакция изменила данные, которые мы читали). EF Core ловит как +`PostgresException`, прокидывает наверх; глобальный middleware возвращает 500 +без тела. + +## Фикс + +Wrap commit + retry. Маленький helper `WithSerializableRetryAsync`: + +```csharp +const int maxAttempts = 5; +for (var attempt = 1; attempt <= maxAttempts; attempt++) { + try { + await using var tx = await _db.Database.BeginTransactionAsync(IsolationLevel.Serializable, ct); + // … work … + await tx.CommitAsync(ct); + return Ok(); + } catch (DbUpdateException ex) when (ex.InnerException is PostgresException pg && pg.SqlState == "40001") { + // Serialization conflict — retry с exp backoff + jitter. + if (attempt == maxAttempts) + return Conflict(new { error = "Слишком много одновременных продаж этого товара. Попробуйте ещё раз." }); + await Task.Delay(TimeSpan.FromMilliseconds(50 * attempt + Random.Shared.Next(50)), ct); + } catch (PostgresException pg) when (pg.SqlState == "40001") { + // Аналогично для исключения вне DbUpdateException обёртки. + if (attempt == maxAttempts) return Conflict(new { error = "Слишком много одновременных продаж этого товара. Попробуйте ещё раз." }); + await Task.Delay(TimeSpan.FromMilliseconds(50 * attempt + Random.Shared.Next(50)), ct); + } +} +``` + +После фикса: те же 20 параллельных постов → большинство retry'ятся и проходят, +остаток возвращает 409 (а не 500) с понятным сообщением. Stock-инвариант +по-прежнему сохраняется (это другое свойство). + +## Retest + +После фикса — те же 20 параллельных POST: +- ok = 10 (полное число доступных единиц) +- 409 = 10 (over-stock или retry-exhausted, с понятным сообщением) +- 500 = 0 ✓ +- stock-invariant: 0 == 0 ✓ diff --git a/tests/e2e/reports/bugs/bug-004-tiny-price-rounded-to-zero.md b/tests/e2e/reports/bugs/bug-004-tiny-price-rounded-to-zero.md new file mode 100644 index 0000000..4f238f9 --- /dev/null +++ b/tests/e2e/reports/bugs/bug-004-tiny-price-rounded-to-zero.md @@ -0,0 +1,51 @@ +# Bug #004 — Очень малая цена (0.0000001) тихо округляется в 0 + +**Severity:** Low (бизнес-логика, не security) +**Component:** `ProductsController.Create/Update/UpdatePrice/BulkUpdate.price-adjust` +**Found:** Sprint 23, 2026-06-08 + +## Воспроизведение + +```bash +POST /api/catalog/products { ..., "prices": [{"priceTypeId":"…", "amount": 0.0000001, "currencyId": "…"}] } +``` + +Ответ: **201 Created**. Перечитываем — `price.amount = 0.0`. + +Контроллер делает `RoundIfNeeded(0.0000001, allowFractional=true)` = +`Math.Round(0.0000001, 2)` = `0.00`. БД-колонка `decimal(18,4)` сохраняет `0`. + +В `FindMissingRequiredPriceAsync` проверка `price.Amount <= 0m` происходит +ДО округления — `0.0000001 > 0`, проходит. После округления — 0, но +required-price-check уже не повторяется. + +## Ожидание + +Один из вариантов: +1. Validator отвергает: «цена меньше 0.01 — введите как минимум 0.01». +2. Округлять ДО проверки required. +3. Документировать: «цены округляются до настроек организации; 0.0000001 → 0». + +## Фикс + +Выбираем (2) — повторно проверить после `RoundIfNeeded`, чтобы required +price check учитывал то что реально окажется в БД. + +```csharp +foreach (var pr in input.Prices ?? []) { + var rounded = RoundIfNeeded(pr.Amount, allowFractional); + if (rounded < 0m) return BadRequest(...); + // Если PriceType.IsRequired — округлённое значение должно быть > 0. +} +// Перевалидируем required AFTER rounding: +if (await FindMissingRequiredPriceAsync(/* rounded */, ct) is { } missing) return BadRequest(...); +``` + +Проще: добавить в `FindMissingRequiredPriceAsync` округление перед сравнением +(или дополнить проверку в начале `Create`/`Update` после Apply). + +## Severity rationale + +Бизнес-проблема (товар с нулевой ценой не попадёт на чек правильно — а +"required" type'ы существуют именно чтобы гарантировать НЕ-нулевую цену), +но эксплоит ограничен авторизованным admin'ом. Не критично, но фиксим.