fix(s23): adversarial bug-hunt — 4 bugs found, all fixed
Some checks are pending
Auto-tag / Create date-tag (push) Waiting to run
CI / Backend (.NET 8) (push) Waiting to run
CI / Web (React + Vite) (push) Waiting to run
CI / POS (WPF, Windows) (push) Waiting to run
Docker API / Build + push API (push) Waiting to run
Docker API / Deploy API on stage (push) Blocked by required conditions

Sprint 23 (adversarial): атаковали систему как недоброжелатель.
Найдено 4 бага, все починены.

Bug #001 (Medium): NULL-byte в Product.Name вызывал 500 без тела.
  Postgres TEXT не принимает \x00. Добавил NoControlChars() в
  ProductInputValidator + CounterpartyInputValidator.

Bug #002 (Low): ProductInputValidator MaximumLength(200) конфликтовал
  со StringLength(500) в DTO и schema HasMaxLength(500). Сделал 500
  везде. Counterparty: 200 → 255 (matches HasMaxLength).

Bug #003 (CRITICAL): параллельные posting'и под Serializable выбрасывали
  PostgresException 40001 → middleware → 500 empty body. Добавил
  SerializationConflictMiddleware который мапит 40001 → 409 Conflict
  с {error, retryable: true}. Также SerializableRetry helper для
  явного retry внутри endpoint'ов с exp backoff. Применил retry-wrap
  к RetailSalesController.Post (PostCoreAsync extracted).

Bug #004 (Low): цена 0.0000001 округлялась до 0 уже после прохождения
  required-price check (check был ДО RoundIfNeeded). FindMissing-
  RequiredPriceAsync теперь округляет перед сравнением — required
  цена реально > 0 после rounding.

Bug reports: tests/e2e/reports/bugs/bug-00[1-4]-*.md (github-issue format).

Multi-tenant attacks (cat 3): clean — все cross-org GET/PUT/DELETE
дают 404, bulk-update affected=0, lists не утекают.
Auth-edge (cat 2): clean — JWT tampering 401, garbage 401, CORS evil.com
не получает allow-origin, fake refresh 400 invalid_grant.
DOS (cat 7): clean — 50MB body 413, 200 headers 431, long URL 200.
Hangfire safety (cat 8): clean — regular Admin → /hangfire 403,
  seed-demo использует tenant context, body org-id игнорируется.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
nns 2026-06-08 01:35:50 +05:00
parent b6f3c55d81
commit 284ad095c1
11 changed files with 379 additions and 5 deletions

26
docs/sprint23-progress.md Normal file
View file

@ -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.

View file

@ -40,10 +40,17 @@ public ProductsController(AppDbContext db, ITenantContext tenant)
.Select(pt => new { pt.Id, pt.Name }) .Select(pt => new { pt.Id, pt.Name })
.ToListAsync(ct); .ToListAsync(ct);
if (required.Count == 0) return null; 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) foreach (var pt in required)
{ {
var price = prices?.FirstOrDefault(p => p.PriceTypeId == pt.Id); 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; return null;
} }

View file

@ -549,7 +549,20 @@ public async Task<IActionResult> Delete(Guid id, CancellationToken ct)
} }
[HttpPost("{id:guid}/post"), RequiresPermission("RetailSalesOperate")] [HttpPost("{id:guid}/post"), RequiresPermission("RetailSalesOperate")]
public async Task<IActionResult> Post(Guid id, CancellationToken ct) public Task<IActionResult> 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<IActionResult>(
ct,
async () => await PostCoreAsync(id, ct),
onExhausted: () => Conflict(new
{
error = "Слишком много одновременных операций с этими товарами. Подождите секунду и попробуйте ещё раз.",
}));
private async Task<IActionResult> PostCoreAsync(Guid id, CancellationToken ct)
{ {
var sale = await _db.RetailSales.Include(s => s.Lines).FirstOrDefaultAsync(s => s.Id == id, ct); var sale = await _db.RetailSales.Include(s => s.Lines).FirstOrDefaultAsync(s => s.Id == id, ct);
if (sale is null) return NotFound(); if (sale is null) return NotFound();

View file

@ -0,0 +1,73 @@
using Microsoft.EntityFrameworkCore;
using Npgsql;
namespace foodmarket.Api.Infrastructure;
/// <summary>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-делегат.
///
/// Использование (в контроллере):
/// <code>
/// 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 = "..." })
/// );
/// </code>
/// </summary>
public static class SerializableRetry
{
/// <summary>Максимум попыток по умолчанию.</summary>
public const int DefaultMaxAttempts = 5;
/// <summary>Запускает <paramref name="action"/> до <paramref name="maxAttempts"/> раз,
/// перехватывая Postgres serialization failures (SqlState=40001). На
/// каждый retry — exp backoff + jitter (50ms × attempt + random 50ms).
/// На исчерпании — вызывает <paramref name="onExhausted"/>.</summary>
public static async Task<TResult> RunAsync<TResult>(
CancellationToken ct,
Func<Task<TResult>> action,
Func<TResult> 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();
}
/// <summary>Проверяет, является ли исключение Postgres serialization
/// failure (включая DbUpdateException-обёртки).</summary>
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;
}
}

View file

@ -0,0 +1,59 @@
using Microsoft.EntityFrameworkCore;
using Npgsql;
namespace foodmarket.Api.Infrastructure;
/// <summary>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'ов).</summary>
public class SerializationConflictMiddleware
{
private readonly RequestDelegate _next;
private readonly ILogger<SerializationConflictMiddleware> _log;
public SerializationConflictMiddleware(RequestDelegate next, ILogger<SerializationConflictMiddleware> 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;
}
}

View file

@ -2,6 +2,21 @@
namespace foodmarket.Api.Infrastructure.Validation; namespace foodmarket.Api.Infrastructure.Validation;
/// <summary>Sprint 23 / bug-001: проверка что строка не содержит NUL-байт
/// и иных control-chars (кроме \t\r\n). Postgres TEXT/VARCHAR отказывается
/// принять \x00, что роняло SaveChanges с 500 без понятного ответа.
/// Применяется на пользовательские input-поля строкового типа.</summary>
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. Зарегистрированы // FluentValidation-валидаторы для основных input-DTO. Зарегистрированы
// автоматически через AddValidatorsFromAssemblyContaining<Program>(). // автоматически через AddValidatorsFromAssemblyContaining<Program>().
@ -66,9 +81,18 @@ public sealed class ProductInputValidator : AbstractValidator<foodmarket.Applica
{ {
public ProductInputValidator() public ProductInputValidator()
{ {
// Sprint 23 / bug-002: было MaximumLength(200), а schema разрешает 500.
// Sprint 23 / bug-001: NoControlChars() — Postgres TEXT отказывается
// принять NUL-byte (DbUpdateException → 500). Проверяем ВСЕ строковые
// поля input'а на control-chars (кроме \t\r\n).
RuleFor(x => x.Name).NotEmpty().WithMessage("Название обязательно.") RuleFor(x => x.Name).NotEmpty().WithMessage("Название обязательно.")
.MaximumLength(200); .MaximumLength(500)
RuleFor(x => x.Article).MaximumLength(50); .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) RuleFor(x => x.UnitOfMeasureId).NotEqual(Guid.Empty)
.WithMessage("Не указана единица измерения."); .WithMessage("Не указана единица измерения.");
RuleFor(x => x.Vat).InclusiveBetween(0m, 100m).When(x => x.Vat.HasValue) RuleFor(x => x.Vat).InclusiveBetween(0m, 100m).When(x => x.Vat.HasValue)
@ -86,8 +110,15 @@ public sealed class CounterpartyInputValidator : AbstractValidator<foodmarket.Ap
{ {
public CounterpartyInputValidator() public CounterpartyInputValidator()
{ {
// Sprint 23 / bug-002: Counterparty.Name column = 255, validator был 200.
// Sprint 23 / bug-001: NoControlChars на всех строковых полях.
RuleFor(x => x.Name).NotEmpty().WithMessage("Название обязательно.") RuleFor(x => 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 цифр. Один из двух // БИН (юрлицо) — 12 цифр, ИИН (физлицо) — 12 цифр. Один из двух
// обязателен только если тип LegalEntity (Type=1). Для Individual (2) // обязателен только если тип LegalEntity (Type=1). Для Individual (2)
// достаточно ИИН или имени. На уровне БД эти ограничения не enforced — // достаточно ИИН или имени. На уровне БД эти ограничения не enforced —

View file

@ -487,6 +487,11 @@ [new Microsoft.OpenApi.Models.OpenApiSecurityScheme
// чтобы они применились даже на 429/403 от rate-limiter'а. // чтобы они применились даже на 429/403 от rate-limiter'а.
app.UseMiddleware<foodmarket.Api.Infrastructure.Security.SecurityHeadersMiddleware>(); app.UseMiddleware<foodmarket.Api.Infrastructure.Security.SecurityHeadersMiddleware>();
// Sprint 23 / bug-003: глобальный перехватчик PostgreSQL serialization
// failure'ов (SqlState=40001) — мапит на 409 Conflict + retryable:true.
// Без него параллельные posting'и под Serializable отвечают 500.
app.UseMiddleware<foodmarket.Api.Infrastructure.SerializationConflictMiddleware>();
app.UseSerilogRequestLogging(); app.UseSerilogRequestLogging();
app.UseCors(CorsPolicy); app.UseCors(CorsPolicy);
// Prometheus HTTP-метрики (http_requests_received_total, http_request_duration_seconds). // Prometheus HTTP-метрики (http_requests_received_total, http_request_duration_seconds).

Binary file not shown.

View file

@ -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.

View file

@ -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 ✓

View file

@ -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'ом. Не критично, но фиксим.