From af12b1ceb4ee1041b63c898aed4a61d986fc92a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=94=D0=BC=D0=B8=D1=82=D1=80=D0=B8=D0=B9?= Date: Tue, 28 Jul 2026 07:21:02 +0300 Subject: [PATCH] =?UTF-8?q?fix=20=D1=80=D0=B5=D0=BA=D0=BB=D0=B0=D0=BC?= =?UTF-8?q?=D0=B0=20=D0=B7=D0=B0=20=D0=BF=D0=BE=D0=BA=D0=B0=D0=B7=D1=8B:?= =?UTF-8?q?=20=D0=BF=D1=80=D0=B5=D0=B4=D0=B5=D0=BB=20=D0=B2=D0=B5=D1=81?= =?UTF-8?q?=D0=B0=20=D0=BA=D0=B0=D1=80=D1=82=D0=B8=D0=BD=D0=BA=D0=B8=20?= =?UTF-8?q?=D0=BF=D1=80=D0=BE=D0=B2=D0=B5=D1=80=D0=B5=D0=BD=20=D0=BF=D0=BE?= =?UTF-8?q?-=D0=BD=D0=B0=D1=81=D1=82=D0=BE=D1=8F=D1=89=D0=B5=D0=BC=D1=83,?= =?UTF-8?q?=20=D0=BE=D0=B1=D1=85=D0=BE=D0=B4=20=D0=BC=D0=BE=D0=B4=D0=B5?= =?UTF-8?q?=D1=80=D0=B0=D1=86=D0=B8=D0=B8=20=D0=BD=D0=B5=20=D1=81=D1=80?= =?UTF-8?q?=D1=8B=D0=B2=D0=B0=D0=B5=D1=82=D1=81=D1=8F=20=D1=86=D0=B5=D0=BB?= =?UTF-8?q?=D0=B8=D0=BA=D0=BE=D0=BC,=20=D1=80=D0=BE=D0=B1=D0=BE=D1=82=20?= =?UTF-8?q?=D0=BD=D0=B5=20=D0=BD=D0=B5=D1=81=D1=91=D1=82=20=D1=82=D0=BE?= =?UTF-8?q?=D0=BA=D0=B5=D0=BD=20=D0=BD=D0=B0=20=D1=87=D1=83=D0=B6=D0=BE?= =?UTF-8?q?=D0=B9=20=D0=B0=D0=B4=D1=80=D0=B5=D1=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Мелочи приёмочного листа v12 §8. Каждая правка с тестом; где защита уже стояла в коде — тест проверен вырезанием этой защиты. Предел веса картинки. Две прежние проверки были пустышками: сравнивали константу саму с собой и с тем же числом в ответе сервера. Вырезание правила max: оставляло обе зелёными. Настоящий тест грузит перевес и ждёт отказа — это четвёртая найденная пустышка за ветку. Обход модерации. Объявление без статуса и причина отказа длиннее колонки роняли запись в базу ВНЕ защиты, и обход обрывался на середине: остальные клиенты не узнавали, приняли их рекламу или отклонили, а деньги за отклонённый набор не возвращались. Запись ответа теперь под той же защитой, что и сеть; пустой статус не пишем вовсе, причину храним обрезанной. Робот. Адрес файла приходил в ответе сервера, а шли по нему со своим токеном без всякой сверки. Теперь адрес обязан вести на портал. Папка снимков экрана росла бесконечно, а на снимках видны логин и остаток счёта — старше двух недель убираются. Ещё: порядок посредников служебного канала — токен раньше служебного соединения; BannerGenerator больше не отдаёт молча файл тяжелее предела и берёт предел из общей константы; нулевой номер креатива ловится намеренно, а не случайно нестрогим сравнением. Портал 300/300, робот 60/60. Мест снятия заморозки денег по-прежнему четыре. Не тронуто намеренно: цена за 1000 показов и бюджет приходят от клиента — но это видимое поле мастера и принятое продуктовое решение, а не недосмотр. Решает владелец. --- app/app/Jobs/SyncCampaignModerationJob.php | 41 ++++-- .../Services/Advertising/BannerGenerator.php | 15 ++- .../Services/Advertising/CampaignLauncher.php | 7 +- app/routes/web.php | 4 +- .../CampaignBannerEndpointsTest.php | 27 ++++ .../Advertising/CampaignLauncherTest.php | 63 +++++++++ .../Advertising/CreativeRobotEndpointTest.php | 18 +++ .../SyncCampaignModerationJobTest.php | 53 ++++++++ .../Unit/Advertising/BannerGeneratorTest.php | 36 +++++- bots/yandex-creatives/README.md | 4 + bots/yandex-creatives/src/portal.js | 25 +++- bots/yandex-creatives/src/runner.js | 29 ++++- bots/yandex-creatives/test/portal.test.js | 32 +++++ bots/yandex-creatives/test/runner.test.js | 29 ++++- .../2026-07-27-PROGRESS-pochinka-v12.md | 120 ++++++++++++++++++ 15 files changed, 480 insertions(+), 23 deletions(-) diff --git a/app/app/Jobs/SyncCampaignModerationJob.php b/app/app/Jobs/SyncCampaignModerationJob.php index 3760f6a7..33c2dd0f 100644 --- a/app/app/Jobs/SyncCampaignModerationJob.php +++ b/app/app/Jobs/SyncCampaignModerationJob.php @@ -66,30 +66,45 @@ class SyncCampaignModerationJob implements ShouldQueue continue; } + // Под защитой не только поход в сеть, но и запись ответа: беда на одной кампании + // не должна срывать обход остальных клиентов. Сорванный обход — это чужая реклама, + // про которую никто не узнал, что её приняли или отклонили, и не вернувшиеся деньги. try { $moderation = $direct->getAdsModeration( $banners->pluck('yandex_ad_id')->map(fn ($v) => (int) $v)->all() ); + + foreach ($banners as $banner) { + $info = $moderation[(int) $banner->yandex_ad_id] ?? null; + if ($info === null) { + continue; + } + + // Яндекс прислал объявление, но без статуса. Пустой статус класть нельзя — + // колонка его не принимает; да и «неизвестно» это не вердикт. Оставляем + // прежний статус: баннер считается ещё не решённым и держит кампанию в ожидании. + $status = $info['status'] ?? null; + if (! is_string($status) || $status === '') { + continue; + } + + // Причина отказа у Яндекса бывает длиннее нашей колонки (модератор перечисляет + // все претензии списком). Храним сколько влезает — клиенту важно начало, + // а потеря причины целиком хуже обрезанной. + $reason = $info['reason'] ?? null; + + $banner->update([ + 'moderation_status' => $status, + 'moderation_reason' => is_string($reason) ? mb_substr($reason, 0, 255) : null, + ]); + } } catch (Throwable $e) { - // Сбой одной кампании не должен валить весь опрос остальных. // ПДн в лог не попадают — только id кампании. Log::warning('SyncCampaignModerationJob: '.$e->getMessage(), ['campaign' => $campaign->id]); continue; } - foreach ($banners as $banner) { - $info = $moderation[(int) $banner->yandex_ad_id] ?? null; - if ($info === null) { - continue; - } - - $banner->update([ - 'moderation_status' => $info['status'] ?? null, - 'moderation_reason' => $info['reason'] ?? null, - ]); - } - // Перечитывать баннеры не нужно: ->update() уже положил новые значения // в ту же модель в памяти. Баннер, про который Яндекс промолчал, остаётся // со своим прежним статусом — то есть считается ещё не решённым и держит diff --git a/app/app/Services/Advertising/BannerGenerator.php b/app/app/Services/Advertising/BannerGenerator.php index 0670921a..771fe492 100644 --- a/app/app/Services/Advertising/BannerGenerator.php +++ b/app/app/Services/Advertising/BannerGenerator.php @@ -14,8 +14,12 @@ use RuntimeException; final class BannerGenerator { /** @return string бинарь JPEG точного размера $width×$height, вес ≤ $maxBytes. */ - public function coverJpeg(string $sourceBinary, int $width, int $height, int $maxBytes = 512000): string + public function coverJpeg(string $sourceBinary, int $width, int $height, ?int $maxBytes = null): string { + // Предел один на весь модуль — потолок Яндекса. Своё число тут уже расходилось бы + // с тем, по которому портал проверяет загрузку клиента. + $maxBytes ??= BannerUploadPolicy::MAX_BYTES; + $src = @imagecreatefromstring($sourceBinary); if ($src === false) { throw new RuntimeException('Не удалось прочитать изображение (ожидались JPG/PNG/GIF).'); @@ -47,6 +51,15 @@ final class BannerGenerator imagedestroy($src); imagedestroy($dst); + // Качество упёрлось в пол, а вес всё равно больше предела. Раньше такой файл уходил + // наружу молча — кабинет Яндекса его не примет, и узнали бы мы об этом уже роботом, + // стоящим перед окном загрузки. Отказ вслух лучше заведомо негодного файла. + if (strlen($bytes) > $maxBytes) { + throw new RuntimeException( + "Картинка не ужимается до {$maxBytes} байт для размера {$width}×{$height} — нужна другая." + ); + } + return $bytes; } } diff --git a/app/app/Services/Advertising/CampaignLauncher.php b/app/app/Services/Advertising/CampaignLauncher.php index 8e763ec5..8fa4c174 100644 --- a/app/app/Services/Advertising/CampaignLauncher.php +++ b/app/app/Services/Advertising/CampaignLauncher.php @@ -155,7 +155,12 @@ final class CampaignLauncher throw new RuntimeException('У кампании нет ни одного включённого баннера — клиенту загрузить картинки в мастере.'); } - $withoutCreative = $banners->firstWhere('yandex_creative_id', null); + // Ноль ловим наравне с пустотой НАМЕРЕННО: это мусор, а не номер. Раньше он попадал + // сюда случайно, нестрогим сравнением; строгое сравнение «по уму» выпустило бы ноль + // дальше — в запрос к Яндексу. + $withoutCreative = $banners->first( + fn ($b) => $b->yandex_creative_id === null || (int) $b->yandex_creative_id === 0 + ); if ($withoutCreative !== null) { throw new RuntimeException( "У баннера {$withoutCreative->width}×{$withoutCreative->height} нет номера креатива Яндекса — креативы ещё не загружены в кабинет." diff --git a/app/routes/web.php b/app/routes/web.php index aa24cbfe..c8bcb9ab 100644 --- a/app/routes/web.php +++ b/app/routes/web.php @@ -364,7 +364,9 @@ Route::middleware(['admin-db', 'sales-integration'])->prefix('api/sales/integrat // Служебный канал «Робот-грузчик креативов → Портал». Токен вместо пользователя. // admin-db — робот ходит без tenant-контекста, работает под ролью crm_admin_user. -Route::middleware(['admin-db', 'creative-robot'])->prefix('api/creative-robot')->group(function () { +// Токен ПЕРВЫМ, служебное соединение вторым: сначала пропуск, потом ключи от служебного +// входа. Тот же порядок, что у админского канала (см. комментарий в UseAdminConnection). +Route::middleware(['creative-robot', 'admin-db'])->prefix('api/creative-robot')->group(function () { Route::get('/next', [CreativeRobotController::class, 'next']); // Номер задания — часть адреса файла: робот получает файлы только того задания, // которое ему выдали, а не «какого-нибудь, что сейчас в работе». diff --git a/app/tests/Feature/Advertising/CampaignBannerEndpointsTest.php b/app/tests/Feature/Advertising/CampaignBannerEndpointsTest.php index 634625ce..12532987 100644 --- a/app/tests/Feature/Advertising/CampaignBannerEndpointsTest.php +++ b/app/tests/Feature/Advertising/CampaignBannerEndpointsTest.php @@ -121,6 +121,33 @@ it('неизвестный размер — 422', function () { expect($res->json('message'))->toBe('Неизвестный размер баннера.'); }); +/** + * Предел веса картинки — 512 000 байт (BannerUploadPolicy::MAX_BYTES), это потолок Яндекса + * для графического креатива. Тяжелее — кабинет файл не примет, и узнаем мы об этом только + * тогда, когда робот уже стоит перед окном загрузки на боевом: он не умеет ни ужать картинку, + * ни спросить клиента. Задание уйдёт в сбой, кампания застрянет. + * + * 🪤 Прежние две проверки про этот предел были пустышками: одна сравнивала константу саму + * с собой, вторая — что то же число приезжает клиенту в JSON. Вырезание правила `max:` + * из валидации оставляло обе зелёными. Этот тест грузит настоящий перевес и проверен + * вырезанием: без правила `max:` он краснеет. + */ +it('картинка тяжелее предела отклоняется', function () { + Storage::fake('local'); + [, $user, $campaign] = bannerCampaign(); + + $tooHeavy = UploadedFile::fake()->image('b.jpg', 728, 90)->size((int) (BannerUploadPolicy::MAX_BYTES / 1024) + 1); + + $res = $this->actingAs($user)->postJson("/api/advertising/campaigns/{$campaign->id}/banners", [ + 'width' => 728, 'height' => 90, + 'file' => $tooHeavy, + ]); + + $res->assertStatus(422); + $res->assertJsonValidationErrors('file'); + expect(AdCampaignBanner::where('campaign_id', $campaign->id)->count())->toBe(0); +}); + it('не-картинка отклоняется валидатором', function () { Storage::fake('local'); [, $user, $campaign] = bannerCampaign(); diff --git a/app/tests/Feature/Advertising/CampaignLauncherTest.php b/app/tests/Feature/Advertising/CampaignLauncherTest.php index 6dbd46d6..ef03dbac 100644 --- a/app/tests/Feature/Advertising/CampaignLauncherTest.php +++ b/app/tests/Feature/Advertising/CampaignLauncherTest.php @@ -903,3 +903,66 @@ it('пока запуск идёт, кампания в базе помечен expect($statusDuringLaunch)->toBe(AdCampaign::STATUS_LAUNCHING) ->and($campaign->fresh()->status)->toBe(AdCampaign::STATUS_PENDING_MODERATION); }); + +/** + * Номер креатива, равный нулю, — это мусор, а не номер. Приезжает он из ручной правки + * или из чужого импорта. Проверка «номера нет» ловила его случайно, нестрогим сравнением + * (в PHP ноль равен пустоте); стоило написать строгое сравнение «по уму» — и ноль поехал бы + * дальше, в запрос к Яндексу. Ловим намеренно и говорим клиенту то же самое, что и про + * отсутствующий номер: картинки в кабинет ещё не отвезли. + */ +it('баннер с нулевым номером креатива считается без номера, а не отправляется в Яндекс', function () { + configureYandex(); + fakeYandexEndpoints(); + + $tenant = Tenant::factory()->create(); + app(AdWalletService::class)->topup($tenant->id, '20000.00', 'yandex', 'тест'); + + $campaign = makeImpressionCampaign($tenant->id); + seedAudience($campaign, 100); + seedBanners($campaign, ['300x250' => 0]); + + expect(fn () => app(CampaignLauncher::class)->launch($campaign)) + ->toThrow(RuntimeException::class, 'нет номера креатива'); + + Http::assertNotSent(fn ($request) => str_contains($request->url(), '/json/v5/ads')); + expect($campaign->fresh()->status)->toBe(AdCampaign::STATUS_DRAFT); +}); + +/** + * Один и тот же номер креатива, вписанный двум баннерам, дал бы два объявления с одной + * картинкой, и хотя бы одно — не своего размера: клиент платит за показы битого баннера. + * + * Отдельной проверки «номера в наборе не повторяются» не нужно: слот «кампания + размер» + * теперь уникален в базе, значит два баннера набора всегда разных размеров, а сверка + * настоящего размера креатива перед созданием объявлений одного из них обязательно поймает. + * Тест держит это рассуждение: сломается сверка — сломается и он. + */ +it('один номер креатива на двух баннерах не создаёт объявлений', function () { + configureYandex(); + Http::fake([ + '*/segments/upload_csv_file' => Http::response(['segment' => ['id' => 900001]]), + '*/segment/*/confirm' => Http::response(['segment' => ['id' => 900001]]), + '*/json/v5/retargetinglists' => Http::response(['result' => ['AddResults' => [['Id' => 111]]]]), + '*/json/v5/campaigns' => Http::response(['result' => ['AddResults' => [['Id' => 222]]]]), + '*/json/v5/adgroups' => Http::response(['result' => ['AddResults' => [['Id' => 333]]]]), + '*/json/v5/audiencetargets' => Http::response(['result' => ['AddResults' => [['Id' => 444]]]]), + '*/json/v5/creatives' => Http::response(['result' => ['Creatives' => [ + ['Id' => 4242, 'Type' => 'HTML5_CREATIVE', 'Width' => 300, 'Height' => 250], + ]]]), + '*/json/v5/ads' => Http::response(['result' => ['AddResults' => [['Id' => 555]]]]), + ]); + + $tenant = Tenant::factory()->create(); + app(AdWalletService::class)->topup($tenant->id, '20000.00', 'yandex', 'тест'); + + $campaign = makeImpressionCampaign($tenant->id); + seedAudience($campaign, 100); + seedBanners($campaign, ['300x250' => 4242, '728x90' => 4242]); + + expect(fn () => app(CampaignLauncher::class)->launch($campaign)) + ->toThrow(RuntimeException::class, 'размер'); + + Http::assertNotSent(fn ($request) => str_contains($request->url(), '/json/v5/ads')); + expect($campaign->fresh()->status)->toBe(AdCampaign::STATUS_DRAFT); +}); diff --git a/app/tests/Feature/Advertising/CreativeRobotEndpointTest.php b/app/tests/Feature/Advertising/CreativeRobotEndpointTest.php index d0ce3c5b..f770584a 100644 --- a/app/tests/Feature/Advertising/CreativeRobotEndpointTest.php +++ b/app/tests/Feature/Advertising/CreativeRobotEndpointTest.php @@ -344,3 +344,21 @@ it('refuses a report for a job already marked failed', function () { expect($job->fresh()->failure_reason)->toBe('первый сбой'); }); + +/** + * Токен проверяем ПЕРВЫМ, а уже потом переключаем соединение с базой на служебную роль. + * Порядок был обратный: запрос без токена сначала переключал соединение и только затем + * получал отказ. Запросов к базе при этом не делалось, поэтому поведение сегодня не меняется — + * это порядок «сначала пропуск, потом ключи от служебного входа», ровно как у админского + * канала (см. комментарий в UseAdminConnection). + * + * Проверить это можно только порядком посредников: наблюдаемой разницы в ответе нет. + * Поэтому тест ничего не гарантирует про поведение — он держит порядок от обратной правки. + */ +it('токен служебного канала проверяется раньше переключения соединения', function () { + $route = collect(app('router')->getRoutes()) + ->first(fn ($r) => $r->uri() === 'api/creative-robot/next'); + + expect($route)->not->toBeNull() + ->and($route->middleware())->toBe(['web', 'creative-robot', 'admin-db']); +}); diff --git a/app/tests/Feature/Advertising/SyncCampaignModerationJobTest.php b/app/tests/Feature/Advertising/SyncCampaignModerationJobTest.php index 46f201eb..cf338119 100644 --- a/app/tests/Feature/Advertising/SyncCampaignModerationJobTest.php +++ b/app/tests/Feature/Advertising/SyncCampaignModerationJobTest.php @@ -121,6 +121,59 @@ it('waits while at least one banner ad is still under moderation', function () { expect($campaign->fresh()->status)->toBe(AdCampaign::STATUS_PENDING_MODERATION); }); +/** + * Обход модерации идёт по ВСЕМ кампаниям всех клиентов подряд. Значит любая беда на одной + * кампании обязана остаться внутри неё: сорвётся обход — остальные клиенты не узнают, что + * их реклама принята или отклонена, а деньги за отклонённый набор не вернутся. + * + * Здесь беда приходит не из сети (это уже прикрыто), а из САМОГО ответа Яндекса: объявление + * пришло без статуса. Запись пустого статуса упирается в запрет базы, и обход обрывается + * на середине. + */ +it('объявление без статуса не срывает обход остальных кампаний', function () { + configureYandexForModeration(); + Http::fake(function ($request) { + $ids = $request->data()['params']['SelectionCriteria']['Ids'] ?? []; + + return Http::response(['result' => ['Ads' => in_array(6001, $ids, true) + ? [['Id' => 6001, 'State' => 'ON']] // статуса нет вовсе + : [['Id' => 6002, 'State' => 'ON', 'StatusClarification' => null, 'Status' => 'ACCEPTED']], + ]]); + }); + + [$broken, $brokenBanners] = makeModeratedCampaignWithBanners([6001], 601); + [$healthy] = makeModeratedCampaignWithBanners([6002], 602); + + (new SyncCampaignModerationJob)->handle(); + + // Кампания без статуса осталась ждать — и не утащила за собой соседнюю. + expect($broken->fresh()->status)->toBe(AdCampaign::STATUS_PENDING_MODERATION) + ->and(AdCampaignBanner::find($brokenBanners[0]->id)->moderation_status)->toBe(AdCampaignBanner::MOD_MODERATION) + ->and($healthy->fresh()->status)->toBe(AdCampaign::STATUS_RUNNING); +}); + +/** + * Причина отказа у Яндекса бывает длинной — там перечисляют все претензии модератора списком. + * В нашей колонке 255 знаков. Длинная причина упирается в базу и обрывает тот же обход. + * Причину показываем клиенту, поэтому храним сколько влезает, а не теряем целиком. + */ +it('слишком длинная причина отказа не срывает обход', function () { + configureYandexForModeration(); + $longReason = str_repeat('Текст на баннере не читается. ', 40); // 1200 знаков + Http::fake(['*/json/v5/ads' => Http::response(['result' => ['Ads' => [ + ['Id' => 6011, 'State' => 'OFF', 'StatusClarification' => $longReason, 'Status' => 'REJECTED'], + ]]])]); + + [$campaign, $banners] = makeModeratedCampaignWithBanners([6011], 611); + + (new SyncCampaignModerationJob)->handle(); + + $reason = AdCampaignBanner::find($banners[0]->id)->moderation_reason; + expect($campaign->fresh()->status)->toBe(AdCampaign::STATUS_REJECTED) + ->and(mb_strlen((string) $reason))->toBe(255) + ->and($reason)->toStartWith('Текст на баннере не читается.'); +}); + // ВЫХОД 2 остаётся ровно один: разморозка зовётся только когда отклонены ВСЕ // объявления набора. Пока живо хоть одно — деньги остаются замороженными. it('не снимает заморозку, пока принято хотя бы одно объявление набора', function () { diff --git a/app/tests/Unit/Advertising/BannerGeneratorTest.php b/app/tests/Unit/Advertising/BannerGeneratorTest.php index b1d7187f..c76e5426 100644 --- a/app/tests/Unit/Advertising/BannerGeneratorTest.php +++ b/app/tests/Unit/Advertising/BannerGeneratorTest.php @@ -18,7 +18,7 @@ function srcJpeg(int $w, int $h): string } it('делает JPEG ТОЧНОГО целевого размера из большой квадратной исходной', function () { - $out = (new BannerGenerator())->coverJpeg(srcJpeg(2000, 2000), 300, 250); + $out = (new BannerGenerator)->coverJpeg(srcJpeg(2000, 2000), 300, 250); $info = getimagesizefromstring($out); expect($info)->not->toBeFalse() @@ -29,7 +29,7 @@ it('делает JPEG ТОЧНОГО целевого размера из бол }); it('работает для широкого и высокого форматов (cover, точные пиксели)', function () { - $gen = new BannerGenerator(); + $gen = new BannerGenerator; $wide = getimagesizefromstring($gen->coverJpeg(srcJpeg(1200, 1200), 728, 90)); expect($wide[0])->toBe(728)->and($wide[1])->toBe(90); @@ -38,7 +38,35 @@ it('работает для широкого и высокого формато expect($tall[0])->toBe(240)->and($tall[1])->toBe(600); }); -it('бросает исключение на нечитаемой картинке', function () { - expect(fn () => (new BannerGenerator())->coverJpeg('не картинка', 300, 250)) +/** Хелпер: «шумная» картинка — такую JPEG не ужимает почти никак, вес остаётся большим. */ +function noisyJpeg(int $w, int $h): string +{ + $img = imagecreatetruecolor($w, $h); + for ($x = 0; $x < $w; $x += 2) { + for ($y = 0; $y < $h; $y += 2) { + imagesetpixel($img, $x, $y, imagecolorallocate($img, ($x * 7) % 255, ($y * 13) % 255, ($x * $y) % 255)); + } + } + ob_start(); + imagejpeg($img, null, 95); + $bin = (string) ob_get_clean(); + imagedestroy($img); + + return $bin; +} + +/** + * Подбор качества упирается в пол 40 и дальше жать не может. Раньше в этом случае наружу + * молча уходил файл любого веса — а кабинет Яндекса тяжелее предела его не примет, и узнали + * бы мы об этом уже роботом, стоящим перед окном загрузки на боевом. Лучше честно сказать, + * что картинка не ужимается, чем отдать заведомо негодный файл. + */ +it('не отдаёт молча файл тяжелее предела, а честно отказывается', function () { + expect(fn () => (new BannerGenerator)->coverJpeg(noisyJpeg(1200, 1200), 970, 250, 2000)) + ->toThrow(RuntimeException::class); +}); + +it('бросает исключение на нечитаемой картинке', function () { + expect(fn () => (new BannerGenerator)->coverJpeg('не картинка', 300, 250)) ->toThrow(RuntimeException::class); }); diff --git a/bots/yandex-creatives/README.md b/bots/yandex-creatives/README.md index 43c4e116..1fd9df77 100644 --- a/bots/yandex-creatives/README.md +++ b/bots/yandex-creatives/README.md @@ -70,6 +70,10 @@ npm test # проверки, браузер не нужен пробует в следующий раз; задание при этом остаётся в очереди. Замок, брошенный убитым процессом (перезагрузка сервера), перехватывается через полчаса. +Снимки экрана в папке `screenshots/` робот убирает сам: старше двух недель — удаляет при +очередном запуске. Свежие нужны, чтобы разобрать вчерашний сбой; лежать вечно им нельзя — +на снимке видна боковая панель кабинета с логином и остатком счёта. + ## Пришло письмо-алярм — что делать В письме указан шаг, на котором робот встал, и приложен снимок экрана. diff --git a/bots/yandex-creatives/src/portal.js b/bots/yandex-creatives/src/portal.js index 0c8bc6e6..3c58e12a 100644 --- a/bots/yandex-creatives/src/portal.js +++ b/bots/yandex-creatives/src/portal.js @@ -36,6 +36,28 @@ function extensionOf(res) { return 'jpg'; } +/** + * Адрес файла приходит В ОТВЕТЕ сервера, а идём мы по нему СО СВОИМ ТОКЕНОМ. Значит адрес + * обязан вести на портал и никуда больше: подменённый или просто перепутанный адрес увёз бы + * ключ от служебного канала на чужую машину. Робот живёт на боевом сервере и ходит по + * внутренней сети, поэтому цена ошибки здесь выше обычной. + */ +function assertPortalUrl(url, portalBaseUrl) { + let target; + try { + target = new URL(String(url)); + } catch { + throw new Error(`Портал прислал непонятный адрес файла: ${url}`); + } + + const portal = new URL(portalBaseUrl); + if (target.origin !== portal.origin) { + throw new Error(`Адрес файла ведёт не на портал, а на ${target.origin} — за файлом не идём`); + } + + return target.href; +} + export function createPortal(config, fetchImpl = fetch) { const headers = { 'X-Creative-Robot-Token': config.robotToken, Accept: 'application/json' }; @@ -58,7 +80,8 @@ export function createPortal(config, fetchImpl = fetch) { * Возвращает путь, по которому файл на самом деле лёг. */ async downloadBanner(banner, targetBase) { - const res = await fetchImpl(banner.file_url, { headers }); + const fileUrl = assertPortalUrl(banner.file_url, config.portalBaseUrl); + const res = await fetchImpl(fileUrl, { headers }); if (!res.ok) throw new Error(`Не скачался файл баннера ${banner.width}x${banner.height}: ${res.status}`); const targetPath = `${targetBase}.${extensionOf(res)}`; diff --git a/bots/yandex-creatives/src/runner.js b/bots/yandex-creatives/src/runner.js index 47394ba5..d00c1e4b 100644 --- a/bots/yandex-creatives/src/runner.js +++ b/bots/yandex-creatives/src/runner.js @@ -1,4 +1,4 @@ -import { existsSync, mkdirSync, rmSync } from 'node:fs'; +import { existsSync, mkdirSync, readdirSync, rmSync, statSync } from 'node:fs'; import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -6,6 +6,32 @@ import { fileURLToPath } from 'node:url'; // каталога: робота запускает расписание, и каталог запуска у него может быть любым. const ROBOT_ROOT = fileURLToPath(new URL('..', import.meta.url)); +/** + * Сколько держим снимки экрана. Свежие нужны — по ним разбирают вчерашний сбой. Старые + * не нужны никому, а лежать им опасно: на снимке видна боковая панель кабинета с логином + * и остатком счёта. Раньше папка не чистилась вовсе и росла бесконечно. + */ +const SCREENSHOT_TTL_MS = 14 * 24 * 60 * 60 * 1000; + +/** Уборка старых снимков. Любая беда здесь молчит: из-за уборки работа встать не должна. */ +function pruneScreenshots(shotsDir, now) { + let names; + try { + names = readdirSync(shotsDir); + } catch { + return; + } + + for (const name of names) { + const path = join(shotsDir, name); + try { + if (now - statSync(path).mtimeMs > SCREENSHOT_TTL_MS) { + rmSync(path, { force: true }); + } + } catch { /* пропал сам или занят — не наша забота */ } + } +} + /** * Один проход робота: спросить работу → скачать файлы → залить в кабинет → отчитаться. * @@ -32,6 +58,7 @@ export async function runOnce(config, portal, browser, mailer, { timestamp, work try { mkdirSync(shotsDir, { recursive: true }); + pruneScreenshots(shotsDir, Date.now()); mkdirSync(dir, { recursive: true }); step = 'скачивание файлов'; diff --git a/bots/yandex-creatives/test/portal.test.js b/bots/yandex-creatives/test/portal.test.js index 06a54338..3aadf59a 100644 --- a/bots/yandex-creatives/test/portal.test.js +++ b/bots/yandex-creatives/test/portal.test.js @@ -117,6 +117,38 @@ test('бросает понятную ошибку с размером банн ); }); +// Адрес файла робот берёт из ответа сервера и идёт по нему СО СВОИМ ТОКЕНОМ. Подменённый +// или просто перепутанный адрес увёл бы ключ от служебного канала на чужой сервер, а робот +// живёт на боевой машине и ходит по внутренней сети. Токен уходит только на портал. +test('не несёт токен на чужой адрес', async () => { + let called = false; + const fetchStub = async () => { called = true; return { ok: true, status: 200, body: null }; }; + + await assert.rejects( + () => createPortal(config, fetchStub).downloadBanner( + { file_url: 'https://evil.example/забрать', banner_id: 1, width: 300, height: 250 }, + join(tmpdir(), 'не-должен-появиться'), + ), + /evil\.example/, + ); + + assert.equal(called, false, 'до запроса дойти не должно — токен не уходит'); +}); + +test('мусор вместо адреса файла тоже отвергается', async () => { + let called = false; + const fetchStub = async () => { called = true; return { ok: true, status: 200, body: null }; }; + + await assert.rejects( + () => createPortal(config, fetchStub).downloadBanner( + { file_url: '/api/creative-robot/jobs/7/banners/1/file', banner_id: 1, width: 300, height: 250 }, + join(tmpdir(), 'не-должен-появиться'), + ), + ); + + assert.equal(called, false); +}); + test('отчитывается об успехе', async () => { const calls = []; const fetchStub = async (url, opts) => { diff --git a/bots/yandex-creatives/test/runner.test.js b/bots/yandex-creatives/test/runner.test.js index 263c444c..cda70091 100644 --- a/bots/yandex-creatives/test/runner.test.js +++ b/bots/yandex-creatives/test/runner.test.js @@ -1,6 +1,6 @@ import test from 'node:test'; import assert from 'node:assert/strict'; -import { existsSync } from 'node:fs'; +import { existsSync, mkdirSync, utimesSync, writeFileSync } from 'node:fs'; import { mkdtemp, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -161,6 +161,33 @@ test('без явной рабочей папки файлы всё равно } }); +/** + * Снимки экрана копились навсегда. Это не просто мусор на диске: на снимке видна боковая + * панель кабинета — логин и остаток счёта. Чем дольше они лежат, тем больше на сервере + * копится того, что там лежать не должно. Свежие нужны — по ним разбирают вчерашний сбой, + * старые не нужны никому. + */ +test('старые снимки экрана убираются, свежие остаются', async () => { + await withWorkDir(async (workDir) => { + const shots = join(workDir, 'screenshots'); + mkdirSync(shots, { recursive: true }); + + const old = join(shots, 'job-1-старый.png'); + const fresh = join(shots, 'job-2-свежий.png'); + writeFileSync(old, 'снимок'); + writeFileSync(fresh, 'снимок'); + + const monthAgo = new Date(Date.now() - 30 * 24 * 60 * 60 * 1000); + utimesSync(old, monthAgo, monthAgo); + + const s = stubs({ job: JOB }); + await runOnce(FAST_CONFIG, s.portal, s.browser, s.mailer, opts(workDir)); + + assert.equal(existsSync(old), false, 'месячной давности снимок должен был уйти'); + assert.equal(existsSync(fresh), true, 'свежий снимок нужен для разбора сбоя'); + }); +}); + test('файлы клиента удаляются при любом исходе — это чужие картинки, копиться им нельзя', async () => { await withWorkDir(async (workDir) => { const s = stubs({ job: JOB, uploadThrows: 'кабинет упал' }); diff --git a/docs/superpowers/2026-07-27-PROGRESS-pochinka-v12.md b/docs/superpowers/2026-07-27-PROGRESS-pochinka-v12.md index 0fa442f8..3c389a2f 100644 --- a/docs/superpowers/2026-07-27-PROGRESS-pochinka-v12.md +++ b/docs/superpowers/2026-07-27-PROGRESS-pochinka-v12.md @@ -425,3 +425,123 @@ v9.07. Чинить не в этой ветке. можно честно прибить к `pgsql_admin`, не подкладывая ей соединение ради тестов. 🪤 Обратная сторона: мутация «сменить соединение на дефолтное» в тестах НЕ покраснеет — это чисто боевая защита, держится на комментарии и на знании про RLS. + +--- + +# Мелочи листа v12 §8 — 28.07.2026 + +Порядок — по опасности. Каждая с тестом; где защита уже стояла в коде, тест проверен +вырезанием этой защиты. + +## Кап веса картинки — была тест-пустышка + +Две прежние проверки про предел 512 000 байт сравнивали константу саму с собой +(`BannerUploadPolicyTest`) и с тем же числом в ответе сервера (`CampaignBannerEndpointsTest`). +Вырезание правила `max:` из валидации оставляло обе зелёными — то есть предел не был +проверен ничем. + +Написан настоящий тест: загрузка картинки правильного размера, но тяжелее предела → 422, +строка баннера не создаётся. Проверен вырезанием: без правила `max:` приходит 201. + +Это **четвёртая** найденная пустышка (после захвата кампании, рабочей папки робота и слепка +при выдаче). Общее у всех: тест не отличал «защита работает» от «защиты нет». + +## Обход модерации срывался целиком из-за одной кампании + +`SyncCampaignModerationJob` записывал ответ Яндекса как есть. Два способа положить весь обход: + +- объявление пришло **без статуса** → в колонку летел пустой статус, а она его не принимает; +- **причина отказа длиннее 255 знаков** (модератор перечисляет претензии списком) → не влезает + в колонку. + +Оба падения были ВНЕ try/catch, который прикрывал только поход в сеть. Сорванный обход — это +чужая реклама, про которую никто не узнал, что её приняли или отклонили, и не вернувшиеся +за отклонённый набор деньги. + +Починено: запись ответа теперь под той же защитой, что и сеть (беда одной кампании не трогает +остальных); пустой статус не пишем вовсе — баннер остаётся «не решённым» и держит кампанию +в ожидании; причину храним обрезанной до 255 знаков. + +Два теста, оба были красными ровно по этим двум отказам базы. + +## Робот нёс токен на адрес, присланный сервером + +`downloadBanner` шёл за файлом по адресу из ответа портала и клал в запрос **свой токен**, +не проверяя, что адрес ведёт на портал. Робот живёт на боевом сервере и ходит по внутренней +сети — цена подменённого или просто перепутанного адреса здесь выше обычной. + +Починено: адрес сверяется с `portalBaseUrl` до запроса; чужой адрес и мусор вместо адреса +дают понятную ошибку. Тесты доказывают, что до запроса дело не доходит вовсе. + +🪤 `URL` переводит кириллическое имя хоста в punycode — первое ожидание теста было написано +неверно, и красный оказался не по делу. Хост в тесте латиницей. + +## Папка снимков экрана росла бесконечно + +`screenshots/` не чистилась никогда. Это не просто мусор: на снимке видна боковая панель +кабинета с логином и остатком счёта. Робот убирает снимки старше двух недель при очередном +запуске; уборка молчит при любой беде — из-за неё работа встать не должна. Описано в README. + +## Порядок посредников служебного канала + +Было `['admin-db','creative-robot']`: запрос без токена сначала переключал соединение +на служебную роль и только потом получал отказ. Стало `['creative-robot','admin-db']` — +сначала пропуск, потом ключи от служебного входа (тот же порядок, что у админского канала). + +⚠️ Наблюдаемой разницы в ответе нет — запросов к базе без токена не делалось. Тест держит +порядок от обратной правки и **ничего не гарантирует про поведение**; так и написано в самом +тесте, чтобы его не приняли за защиту. + +## BannerGenerator молча отдавал файл любого веса + +Подбор качества упирался в пол 40 и возвращал что получилось. Кабинет Яндекса такой файл +не примет, и узнали бы мы об этом уже роботом, стоящим перед окном загрузки на боевом. +Теперь — понятный отказ. Заодно убран захардкоженный `512000` в сигнатуре: предел берётся +из `BannerUploadPolicy::MAX_BYTES`, иначе своё число тут разошлось бы с тем, по которому +портал проверяет загрузку клиента. + +## Нулевой номер креатива + +`firstWhere('yandex_creative_id', null)` ловил ноль **случайно** — нестрогим сравнением. +Написали бы строгое «по уму» — и ноль поехал бы в запрос к Яндексу. Теперь ноль ловится +намеренно, рядом с пустотой, и об этом сказано в комментарии. Проверено вырезанием: без +проверки на ноль запуск всё равно останавливается, но уже ПОСЛЕ обращения к Яндексу и +с невнятным для клиента текстом. + +## Повтор одного номера креатива в наборе — закрыто побочно + +Отдельная проверка не нужна: слот «кампания + размер» теперь уникален в базе (v9.09), значит +два баннера набора всегда разных размеров, а сверка настоящего размера креатива перед +созданием объявлений (П4) один из них обязательно поймает. Рассуждение закреплено тестом: +сломается сверка — сломается и он. + +## Закрыто побочно ещё раньше + +`Width`/`Height` без `?? 0` — во всех трёх местах `YandexDirectClient` уже с защитой +(правки П4/П6). + +## 🔴 НЕ трогал — вопрос владельцу + +`client_cpm_rub` и `budget_rub` приходят от клиента. В листе v12 это записано как «лучше +не отдавать клиенту», но в мастере кампании стоит **видимое поле «Ваша цена за 1000 показов, ₽»** +(`CampaignWizard.vue`, `data-testid="client-cpm-input"`), и в коде прямо написано: +«Предзаполняем цену дефолтом сервера ровно один раз — дальше клиент правит сам». + +То есть это не недосмотр, а принятое продуктовое решение: клиент назначает свою ставку. +`budget_rub` — счётная величина от сервера, которую клиент видит в списке кампаний. +Убрать их — значит менять продукт и ломать мастер. **Решает владелец.** + +## Зелёная отметка после мелочей + +- портал: **300/300, 1029 проверок, ~48 с**; +- робот: **60/60**; +- вызовов снятия заморозки — **четыре**, пятого не появилось. + +## Осталось из §8 + +- `isDisabled` у кнопки «Создать» — проверять на первом живом прогоне (задача 17), заранее + никак. +- `CampaignBannerService` и `BannerGenerator` — мёртвый продуктовый код, удалять только + с разрешения владельца. +- `db/schema.sql` без рекламных таблиц и устаревшая шапка `db/CHANGELOG_schema.md` — + отдельный canon-sync, не в этой ветке.