Enhance JWT signing key validation and improve FfmpegBumperRenderer label handling: add checks for signing key length and placeholder values in DependencyInjection, and refactor label processing to read from files in FfmpegBumperRenderer. Update MediaProcessingBackgroundService to ensure slot release on claim failure.
This commit is contained in:
@@ -0,0 +1,88 @@
|
|||||||
|
# Ревью TeleWave — план исправлений
|
||||||
|
|
||||||
|
Статус чекбоксов: `[ ]` не сделано · `[x]` исправлено · `[~]` в работе.
|
||||||
|
|
||||||
|
Дата ревью: 2026-07-25. Область: backend (C#/.NET 10), frontend (React 19), инфра.
|
||||||
|
Сборка чистая, 144 теста зелёные, typecheck чистый.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 🔴 Критичное
|
||||||
|
|
||||||
|
- [x] **C1. Утечка слота семафора → медиа-обработка навсегда встаёт**
|
||||||
|
`MediaProcessingBackgroundService.cs:52` — слот берётся до `ClaimNextAsync`; бросок из claim
|
||||||
|
уводит во внешний `catch` без `Release()`. После N ошибок диспетчер зависает навсегда.
|
||||||
|
_Fix: try/finally вокруг claim либо Release() в ветке ошибки._
|
||||||
|
|
||||||
|
- [x] **C2. Слабый JWT-ключ по умолчанию без fail-fast**
|
||||||
|
`appsettings.json:8` + `DependencyInjection.cs:65` — placeholder-ключ, нет проверки на
|
||||||
|
переопределение и длину; тот же ключ у stream-токенов. Забыли → обход авторизации.
|
||||||
|
_Fix: на старте кидать, если ключ = placeholder или < 32 байт._
|
||||||
|
|
||||||
|
- [x] **C3. Инъекция/поломка ffmpeg-фильтра из пользовательских подписей**
|
||||||
|
`FfmpegBumperRenderer.cs:306` — `EscapeText` не экранирует `,` `;` `[` `]` `%` перевод строки.
|
||||||
|
Подпись с запятой рвёт filtergraph; `[`/`]` — инъекция звеньев. Валидатор проверяет только длину.
|
||||||
|
_Fix: экранировать полный набор метасимволов filtergraph._
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 🟠 Среднее
|
||||||
|
|
||||||
|
### Безопасность
|
||||||
|
- [ ] **M1. Cookie без `Secure` за TLS-прокси** — `AuthEndpoints.cs:207`, `StreamingEndpoints.cs:63`.
|
||||||
|
- [ ] **M2. Ключи TMDb/OMDb в логах** — `TmdbMetadataProvider.cs:27`, `OmdbMetadataProvider.cs:25` (`System.Net.Http` не приглушён в Serilog).
|
||||||
|
- [ ] **M3. Stream-токен неотзывной, TTL 6ч, игнорирует userId/блокировку** — `StreamTokenService.cs:15`, `StreamingEndpoints.cs:98,123`.
|
||||||
|
- [ ] **M4. Rate-limiter глобальный (не партиционирован), только на `/api/auth`** — `Program.cs:66`.
|
||||||
|
|
||||||
|
### Архитектура / транзакции
|
||||||
|
- [ ] **M5. `ExecuteDeleteAsync` ломает границу UnitOfWork + файлы удаляются до коммита** — `DeleteShowMediaCommandHandler.cs:36`, `ClearAllMediaCommandHandler.cs`, `DeleteAllShowsCommandHandler.cs`.
|
||||||
|
- [ ] **M6. Query с побочным эффектом на ФС** — `RenderBumperPreviewQueryHandler.cs:18` (рендер файлов под видом запроса).
|
||||||
|
- [ ] **M7. Файловый/HTTP I/O до коммита** — Delete/Register/RefreshEpisodes хендлеры (orphan-файлы при откате).
|
||||||
|
|
||||||
|
### Планировщик / медиа
|
||||||
|
- [ ] **M8. Нет таймаута на ffmpeg/ffprobe** — `ProcessRunner.cs:63` (зависший процесс держит слот).
|
||||||
|
- [ ] **M9. Гонка при конкурентной генерации расписания канала** — `ScheduleGenerator.cs:62` (тик + regenerate → дубли записей).
|
||||||
|
- [ ] **M10. Рендер заставок синхронно внутри тика планировщика** — `ScheduleBumperResolver.cs:174`.
|
||||||
|
- [ ] **M11. Override через полночь не работает** — `SchedulePlannerModels.cs:40` (`EndMinute <= StartMinute` → пустое окно).
|
||||||
|
|
||||||
|
### Фронтенд
|
||||||
|
- [ ] **M12. Молчаливое проглатывание ошибки → вечный скелетон** — `AirPage.tsx:43`.
|
||||||
|
- [ ] **M13. Повторный 401 после refresh не разлогинивает** — `client.ts:80`.
|
||||||
|
- [ ] **M14. Клиентская пагинация поверх усечённого ответа** — `ShowDetail.tsx:42` (`pageSize:500`), `ChannelDetail.tsx:32` (`pageSize:100`).
|
||||||
|
- [ ] **M15. Дедуп загрузок только по имени файла** — `upload-store.ts:143`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 🟡 Низкое / прочее
|
||||||
|
|
||||||
|
- [ ] **L1. Синхронный I/O без CancellationToken в портах удаления** (`IMediaStorage`/`IImageStore`/`IBumperTemplateStorage`).
|
||||||
|
- [ ] **L2. `ValidationBehavior` вызывает `Validate` вместо `ValidateAsync`** — `ValidationBehavior.cs:24`.
|
||||||
|
- [ ] **L3. Инвариант «Single = 1 серия» проверяется в хендлере, а не в агрегате** — `Show.AddEpisode`.
|
||||||
|
- [ ] **L4. Двойной `SaveChanges` в командном пути ScheduleGenerator** — `ScheduleGenerator.cs:79,132`.
|
||||||
|
- [ ] **L5. HLS-плеер не восстанавливается после fatal network/media error** — `ChannelPlayer.tsx:106`.
|
||||||
|
- [ ] **L6. Бесконечная повторная регистрация «падающего» файла из inbox** — `InboxScannerBackgroundService.cs:92`.
|
||||||
|
- [ ] **L7. Возможное переполнение int в весах шоу** — `SchedulePlanner.cs:256,265`.
|
||||||
|
- [ ] **L8. OpenAPI/Scalar мапятся всегда, без гейта по окружению** — `Program.cs:108`.
|
||||||
|
- [ ] **L9. `AllowedHosts: "*"` и dev-креды БД в appsettings.json**.
|
||||||
|
- [ ] **L10. SSRF-поверхность в ImageDownloader** (без allowlist схемы/хоста) — `ImageDownloader.cs:16`.
|
||||||
|
- [ ] **L11. Документация (CLAUDE.md/README) отстала**: заявлено «каталог каналов и раздача видео не реализованы», хотя реализованы.
|
||||||
|
- [ ] **L12. Пробелы в тестах**: фоновые сервисы, ScheduleGenerator-оркестрация, media-конвейер, эндпоинты без тестов.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Журнал исправлений
|
||||||
|
|
||||||
|
### 2026-07-25 — критические (C1–C3)
|
||||||
|
|
||||||
|
- **C1** — `MediaProcessingBackgroundService.cs`: `ClaimNextAsync` обёрнут в `try/catch`, слот
|
||||||
|
освобождается (`slots.Release()`) при любом сбое захвата перед `throw`. Теперь транзиентная
|
||||||
|
ошибка БД не «съедает» слот семафора — диспетчер не зависает.
|
||||||
|
- **C2** — `Infrastructure/DependencyInjection.cs`: после загрузки `JwtOptions` добавлен fail-fast:
|
||||||
|
старт падает с понятным сообщением, если `Jwt:SigningKey` короче 32 байт или содержит `change-me`
|
||||||
|
(значение-заглушка). Тот же ключ подписывает stream-токены, поэтому это закрывает и их.
|
||||||
|
- **C3** — `FfmpegBumperRenderer.cs`: подписи «Сейчас/Далее» (`NowLabel`/`NextLabel`) переведены с
|
||||||
|
инлайнового `text=` на `textfile=` (`nowlabel.txt`/`nextlabel.txt`, `expansion=none`), как уже
|
||||||
|
сделано для названий шоу. Метасимволы filtergraph в подписи больше не ломают/не инъектируют
|
||||||
|
цепочку. Неиспользуемый `EscapeText` удалён. Файлы чистятся в `finally`.
|
||||||
|
|
||||||
|
Проверка: `dotnet build` — 0 warnings/0 errors (при `TreatWarningsAsErrors`); тесты 95 + 49 зелёные.
|
||||||
@@ -66,6 +66,19 @@ public static class DependencyInjection
|
|||||||
configuration.GetSection(JwtOptions.SectionName).Get<JwtOptions>()
|
configuration.GetSection(JwtOptions.SectionName).Get<JwtOptions>()
|
||||||
?? throw new InvalidOperationException("Секция конфигурации 'Jwt' не задана.");
|
?? throw new InvalidOperationException("Секция конфигурации 'Jwt' не задана.");
|
||||||
|
|
||||||
|
// Fail-fast на подписывающем ключе: этот же ключ подписывает и JWT, и stream-токены
|
||||||
|
// (StreamTokenService), поэтому placeholder/короткий ключ из appsettings.json = полный обход
|
||||||
|
// авторизации (можно сфорджить admin-JWT). Лучше не стартовать вовсе, чем стартовать уязвимым.
|
||||||
|
// HMAC-SHA256 требует ключ не короче размера хеша (32 байта), иначе он ослаблен нулевым паддингом.
|
||||||
|
if (Encoding.UTF8.GetByteCount(jwtOptions.SigningKey) < 32)
|
||||||
|
throw new InvalidOperationException(
|
||||||
|
"Jwt:SigningKey должен быть не короче 32 байт. Задайте криптостойкий секрет через конфигурацию/переменную окружения Jwt__SigningKey."
|
||||||
|
);
|
||||||
|
if (jwtOptions.SigningKey.Contains("change-me", StringComparison.OrdinalIgnoreCase))
|
||||||
|
throw new InvalidOperationException(
|
||||||
|
"Jwt:SigningKey использует значение-заглушку из appsettings.json. Переопределите его криптостойким секретом (Jwt__SigningKey)."
|
||||||
|
);
|
||||||
|
|
||||||
services
|
services
|
||||||
.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
|
.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
|
||||||
.AddJwtBearer(options =>
|
.AddJwtBearer(options =>
|
||||||
|
|||||||
@@ -44,9 +44,40 @@ public sealed class FfmpegBumperRenderer(
|
|||||||
await File.WriteAllTextAsync(nowFile, line1, new UTF8Encoding(false), cancellationToken);
|
await File.WriteAllTextAsync(nowFile, line1, new UTF8Encoding(false), cancellationToken);
|
||||||
await File.WriteAllTextAsync(nextFile, line2, new UTF8Encoding(false), cancellationToken);
|
await File.WriteAllTextAsync(nextFile, line2, new UTF8Encoding(false), cancellationToken);
|
||||||
|
|
||||||
|
// Подписи «Сейчас/Далее» тоже пользователь-редактируемы (валидатор ограничивает только длину),
|
||||||
|
// поэтому их так же читаем через textfile=, а не подставляем в text= инлайн: иначе запятая/`;`/`[`/`]`
|
||||||
|
// в подписи ломают (или инъектируют звенья в) цепочку -filter_complex. Нужны лишь в режиме
|
||||||
|
// «Сейчас/Далее» (не FreeText), где рисуются подписи.
|
||||||
|
var nowLabelFile = Path.Combine(assetDir, "nowlabel.txt");
|
||||||
|
var nextLabelFile = Path.Combine(assetDir, "nextlabel.txt");
|
||||||
|
if (!spec.FreeText)
|
||||||
|
{
|
||||||
|
await File.WriteAllTextAsync(
|
||||||
|
nowLabelFile,
|
||||||
|
spec.NowLabel,
|
||||||
|
new UTF8Encoding(false),
|
||||||
|
cancellationToken
|
||||||
|
);
|
||||||
|
await File.WriteAllTextAsync(
|
||||||
|
nextLabelFile,
|
||||||
|
spec.NextLabel,
|
||||||
|
new UTF8Encoding(false),
|
||||||
|
cancellationToken
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
var args = BuildArgs(assetDir, seg, target, nowFile, nextFile, spec);
|
var args = BuildArgs(
|
||||||
|
assetDir,
|
||||||
|
seg,
|
||||||
|
target,
|
||||||
|
nowFile,
|
||||||
|
nextFile,
|
||||||
|
nowLabelFile,
|
||||||
|
nextLabelFile,
|
||||||
|
spec
|
||||||
|
);
|
||||||
var result = await ProcessRunner.RunAsync(
|
var result = await ProcessRunner.RunAsync(
|
||||||
_media.FfmpegPath,
|
_media.FfmpegPath,
|
||||||
args,
|
args,
|
||||||
@@ -83,6 +114,8 @@ public sealed class FfmpegBumperRenderer(
|
|||||||
{
|
{
|
||||||
TryDelete(nowFile);
|
TryDelete(nowFile);
|
||||||
TryDelete(nextFile);
|
TryDelete(nextFile);
|
||||||
|
TryDelete(nowLabelFile);
|
||||||
|
TryDelete(nextLabelFile);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -92,6 +125,8 @@ public sealed class FfmpegBumperRenderer(
|
|||||||
int target,
|
int target,
|
||||||
string nowFile,
|
string nowFile,
|
||||||
string nextFile,
|
string nextFile,
|
||||||
|
string nowLabelFile,
|
||||||
|
string nextLabelFile,
|
||||||
BumperRenderSpec spec
|
BumperRenderSpec spec
|
||||||
)
|
)
|
||||||
{
|
{
|
||||||
@@ -186,7 +221,7 @@ public sealed class FfmpegBumperRenderer(
|
|||||||
vchain
|
vchain
|
||||||
.Append(',')
|
.Append(',')
|
||||||
.Append(
|
.Append(
|
||||||
DrawLabel(font, spec.NowLabel, spec.AccentColor, labelSize, nowLabelY, 0.2)
|
DrawLabel(font, nowLabelFile, spec.AccentColor, labelSize, nowLabelY, 0.2)
|
||||||
);
|
);
|
||||||
vchain
|
vchain
|
||||||
.Append(',')
|
.Append(',')
|
||||||
@@ -194,7 +229,7 @@ public sealed class FfmpegBumperRenderer(
|
|||||||
vchain
|
vchain
|
||||||
.Append(',')
|
.Append(',')
|
||||||
.Append(
|
.Append(
|
||||||
DrawLabel(font, spec.NextLabel, spec.AccentColor, labelSize, nextLabelY, 1.0)
|
DrawLabel(font, nextLabelFile, spec.AccentColor, labelSize, nextLabelY, 1.0)
|
||||||
);
|
);
|
||||||
vchain
|
vchain
|
||||||
.Append(',')
|
.Append(',')
|
||||||
@@ -284,13 +319,15 @@ public sealed class FfmpegBumperRenderer(
|
|||||||
|
|
||||||
private static string DrawLabel(
|
private static string DrawLabel(
|
||||||
string font,
|
string font,
|
||||||
string text,
|
string textFile,
|
||||||
string color,
|
string color,
|
||||||
int size,
|
int size,
|
||||||
int y,
|
int y,
|
||||||
double fadeStart
|
double fadeStart
|
||||||
) =>
|
) =>
|
||||||
$"drawtext=fontfile={font}:text={EscapeText(text)}:expansion=none"
|
// Подпись читается из файла (textfile=) с expansion=none — произвольные символы подписи
|
||||||
|
// не могут сломать/инъектировать цепочку filter_complex (см. запись файлов в RenderAsync).
|
||||||
|
$"drawtext=fontfile={font}:textfile={EscapePath(textFile)}:expansion=none"
|
||||||
+ $":fontcolor={color}:fontsize={size}:x=(w-text_w)/2:y={y}"
|
+ $":fontcolor={color}:fontsize={size}:x=(w-text_w)/2:y={y}"
|
||||||
+ ":shadowcolor=black@0.6:shadowx=1:shadowy=1"
|
+ ":shadowcolor=black@0.6:shadowx=1:shadowy=1"
|
||||||
+ $":alpha='{FadeExpr(fadeStart)}'";
|
+ $":alpha='{FadeExpr(fadeStart)}'";
|
||||||
@@ -302,10 +339,6 @@ public sealed class FfmpegBumperRenderer(
|
|||||||
/// двоеточие экранируется). На Linux (контейнере) — фактически no-op.</summary>
|
/// двоеточие экранируется). На Linux (контейнере) — фактически no-op.</summary>
|
||||||
private static string EscapePath(string path) => path.Replace('\\', '/').Replace(":", "\\:");
|
private static string EscapePath(string path) => path.Replace('\\', '/').Replace(":", "\\:");
|
||||||
|
|
||||||
/// <summary>Экранирование литерального текста подписи внутри значения опции drawtext.</summary>
|
|
||||||
private static string EscapeText(string text) =>
|
|
||||||
text.Replace("\\", "\\\\").Replace(":", "\\:").Replace("'", "\\'");
|
|
||||||
|
|
||||||
private static string Fmt(double value) =>
|
private static string Fmt(double value) =>
|
||||||
value.ToString("0.###", CultureInfo.InvariantCulture);
|
value.ToString("0.###", CultureInfo.InvariantCulture);
|
||||||
|
|
||||||
|
|||||||
@@ -50,7 +50,21 @@ public sealed class MediaProcessingBackgroundService(
|
|||||||
while (!stoppingToken.IsCancellationRequested)
|
while (!stoppingToken.IsCancellationRequested)
|
||||||
{
|
{
|
||||||
await slots.WaitAsync(stoppingToken);
|
await slots.WaitAsync(stoppingToken);
|
||||||
var claim = await ClaimNextAsync(stoppingToken);
|
|
||||||
|
// Слот уже захвачен — любой сбой захвата ассета (транзиентная ошибка БД и т.п.)
|
||||||
|
// обязан вернуть слот, иначе после нескольких ошибок семафор исчерпается и
|
||||||
|
// диспетчер зависнет навсегда (сервис формально жив, но ничего не обрабатывает).
|
||||||
|
(Guid Id, string Extension)? claim;
|
||||||
|
try
|
||||||
|
{
|
||||||
|
claim = await ClaimNextAsync(stoppingToken);
|
||||||
|
}
|
||||||
|
catch
|
||||||
|
{
|
||||||
|
slots.Release();
|
||||||
|
throw;
|
||||||
|
}
|
||||||
|
|
||||||
if (claim is not { } job)
|
if (claim is not { } job)
|
||||||
{
|
{
|
||||||
slots.Release();
|
slots.Release();
|
||||||
|
|||||||
Reference in New Issue
Block a user