From fb4e711b4a25733f49443916987164fef8e69637 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: Sat, 23 May 2026 10:16:46 +0300 Subject: [PATCH] =?UTF-8?q?fix(rls):=20close=204=20dev=E2=86=94prod=20RLS?= =?UTF-8?q?=20gaps=20in=20cron/jobs=20(hole=20#7=20Phase=20B)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by docs/audit/2026-05-23-rls-gap-audit.md. Each touched an RLS-protected table on the default connection in cron/queue context (no tenant GUC) — crash or silent misbehaviour on prod (crm_app_user, not BYPASSRLS), hidden on dev (superuser). - RemindersDispatchDue (Pattern B): gather pending via pgsql_supplier, then per-reminder DB::transaction + SET LOCAL app.current_tenant_id (isolation kept). - ReportsCleanupExpired (Pattern A): SaaS-admin cron → report_jobs + pd_processing_log via pgsql_supplier (BYPASSRLS). - GenerateReportJob (Pattern B): +readonly int $tenantId ctor param, wrap handle() in DB::transaction + SET LOCAL; both ReportJobController dispatch sites updated. - ProcessWebhookJob::failed (Pattern A): failed_webhook_jobs insert via pgsql_supplier → webhook failures now logged, incidents:watch-failures can see them. Tests +SharesSupplierPdo trait. 118 passed / 0 failed. My 5 src files pass larastan isolated (0 errors). --- .../Console/Commands/RemindersDispatchDue.php | 34 +++--- .../Commands/ReportsCleanupExpired.php | 53 +++++---- .../Controllers/Api/ReportJobController.php | 4 +- app/app/Jobs/GenerateReportJob.php | 103 ++++++++++-------- app/app/Jobs/ProcessWebhookJob.php | 6 +- app/tests/Feature/ProcessWebhookJobTest.php | 5 + .../Feature/RemindersDispatchDueTest.php | 2 + .../Feature/Reports/ReportLifecycleTest.php | 2 + 8 files changed, 126 insertions(+), 83 deletions(-) diff --git a/app/app/Console/Commands/RemindersDispatchDue.php b/app/app/Console/Commands/RemindersDispatchDue.php index 32bbb357..8200561f 100644 --- a/app/app/Console/Commands/RemindersDispatchDue.php +++ b/app/app/Console/Commands/RemindersDispatchDue.php @@ -45,9 +45,13 @@ class RemindersDispatchDue extends Command $limit = max(1, (int) $this->option('limit')); $now = Carbon::now(); - // Берём список pending-reminders. Без RLS — admin-flow на serverside. - // Для каждой устанавливаем app.current_tenant_id внутри транзакции. - $pending = Reminder::query() + // Cross-tenant gather via BYPASSRLS connection — on prod crm_app_user cannot + // call current_setting('app.current_tenant_id') without a GUC set first. + // pgsql_supplier (crm_supplier_worker, BYPASSRLS) is the canonical pattern + // for SaaS-admin cron queries (precedent: IncidentsWatchFailures, Reset*). + $rows = DB::connection('pgsql_supplier') + ->table('reminders') + ->select(['id', 'tenant_id', 'deal_id', 'remind_at']) ->where('is_sent', false) ->whereNull('completed_at') ->where('remind_at', '<=', $now) @@ -55,7 +59,7 @@ class RemindersDispatchDue extends Command ->limit($limit) ->get(); - if ($pending->isEmpty()) { + if ($rows->isEmpty()) { $this->info('Нет due-reminders.'); return self::SUCCESS; @@ -64,22 +68,26 @@ class RemindersDispatchDue extends Command $sent = 0; $failed = 0; - foreach ($pending as $reminder) { + foreach ($rows as $row) { if ($dryRun) { $this->line(sprintf( ' would dispatch id=%d tenant=%d deal=%d remind_at=%s', - $reminder->id, - $reminder->tenant_id, - $reminder->deal_id, - $reminder->remind_at?->toIso8601String() ?? '-', + $row->id, + $row->tenant_id, + $row->deal_id, + $row->remind_at ?? '-', )); continue; } try { - DB::transaction(function () use ($reminder, $service): void { - DB::statement('SET LOCAL app.current_tenant_id = '.(int) $reminder->tenant_id); + DB::transaction(function () use ($row, $service): void { + // SET LOCAL scopes GUC to this transaction — PgBouncer-safe. + DB::statement('SET LOCAL app.current_tenant_id = '.(int) $row->tenant_id); + // Fetch the full Eloquent model with tenant context active so + // relations (user, etc.) work correctly inside NotificationService. + $reminder = Reminder::query()->findOrFail((int) $row->id); $service->notifyReminder($reminder); $reminder->update([ 'is_sent' => true, @@ -87,10 +95,10 @@ class RemindersDispatchDue extends Command ]); }); $sent++; - $this->info(" dispatched id={$reminder->id}"); + $this->info(" dispatched id={$row->id}"); } catch (\Throwable $e) { $failed++; - $this->error(" failed id={$reminder->id}: {$e->getMessage()}"); + $this->error(" failed id={$row->id}: {$e->getMessage()}"); } } diff --git a/app/app/Console/Commands/ReportsCleanupExpired.php b/app/app/Console/Commands/ReportsCleanupExpired.php index 22378daa..bcebe9cd 100644 --- a/app/app/Console/Commands/ReportsCleanupExpired.php +++ b/app/app/Console/Commands/ReportsCleanupExpired.php @@ -5,9 +5,9 @@ declare(strict_types=1); namespace App\Console\Commands; use App\Models\ReportJob; -use App\Services\Pd\PdAuditLogger; use Illuminate\Console\Command; use Illuminate\Support\Carbon; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Storage; /** @@ -43,7 +43,11 @@ class ReportsCleanupExpired extends Command $dryRun = (bool) $this->option('dry-run'); $limit = (int) $this->option('limit'); - $jobs = ReportJob::query() + // Cross-tenant gather via BYPASSRLS connection — crm_app_user on prod cannot + // evaluate current_setting('app.current_tenant_id') without a GUC set. + $rows = DB::connection('pgsql_supplier') + ->table('report_jobs') + ->select(['id', 'tenant_id', 'file_path', 'expires_at']) ->where('status', ReportJob::STATUS_DONE) ->whereNotNull('file_path') ->where('expires_at', '<', Carbon::now()) @@ -51,36 +55,45 @@ class ReportsCleanupExpired extends Command ->limit($limit) ->get(); - if ($jobs->isEmpty()) { + if ($rows->isEmpty()) { $this->info('Нет expired report-files для удаления.'); return self::SUCCESS; } $count = 0; - foreach ($jobs as $job) { + foreach ($rows as $row) { $this->line(sprintf( '[%s] tenant=%d job=%d path=%s expired_at=%s', $dryRun ? 'DRY' : 'DEL', - $job->tenant_id, - $job->id, - $job->file_path, - $job->expires_at?->toIso8601String() ?? '?', + $row->tenant_id, + $row->id, + $row->file_path, + $row->expires_at ?? '?', )); if (! $dryRun) { - Storage::disk('local')->delete($job->file_path); - app(PdAuditLogger::class)->record( - action: 'deleted', - subjectType: 'lead', - subjectId: null, - purpose: 'report_cleanup_expired_'.$job->id, - tenantId: $job->tenant_id, - actorTenantUserId: null, - actorAdminUserId: null, - ip: null, - ); - $job->update(['file_path' => null]); + Storage::disk('local')->delete($row->file_path); + + // Both writes go through pgsql_supplier (BYPASSRLS) — this is a + // SaaS-admin cron, not a per-user action, so no tenant GUC is + // required. Same pattern as IncidentsWatchFailures, Reset*. + DB::connection('pgsql_supplier')->table('pd_processing_log')->insert([ + 'tenant_id' => $row->tenant_id, + 'subject_type' => 'lead', + 'subject_id' => null, + 'action' => 'deleted', + 'purpose' => 'report_cleanup_expired_'.$row->id, + 'actor_tenant_user_id' => null, + 'actor_admin_user_id' => null, + 'ip_address' => null, + 'created_at' => now(), + ]); + + DB::connection('pgsql_supplier') + ->table('report_jobs') + ->where('id', $row->id) + ->update(['file_path' => null]); } $count++; } diff --git a/app/app/Http/Controllers/Api/ReportJobController.php b/app/app/Http/Controllers/Api/ReportJobController.php index 291cc738..d5e04303 100644 --- a/app/app/Http/Controllers/Api/ReportJobController.php +++ b/app/app/Http/Controllers/Api/ReportJobController.php @@ -173,7 +173,7 @@ class ReportJobController extends Controller // Sync queue на dev — Job выполняется немедленно. // На prod queue.driver=redis/database — async через worker. - GenerateReportJob::dispatch($job->id); + GenerateReportJob::dispatch($job->id, (int) $user->tenant_id); return response()->json([ 'job' => $this->toResource($job->fresh()), @@ -254,7 +254,7 @@ class ReportJobController extends Controller 'status' => ReportJob::STATUS_PENDING, ]); - GenerateReportJob::dispatch($newJob->id); + GenerateReportJob::dispatch($newJob->id, (int) $user->tenant_id); return response()->json([ 'job' => $this->toResource($newJob->fresh()), diff --git a/app/app/Jobs/GenerateReportJob.php b/app/app/Jobs/GenerateReportJob.php index e5dabcf2..affa6544 100644 --- a/app/app/Jobs/GenerateReportJob.php +++ b/app/app/Jobs/GenerateReportJob.php @@ -12,6 +12,7 @@ use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; use Illuminate\Support\Carbon; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; use Throwable; @@ -37,65 +38,73 @@ class GenerateReportJob implements ShouldQueue public function __construct( public readonly int $reportJobId, + public readonly int $tenantId, ) {} public function handle(ReportGeneratorRegistry $registry): void { - $job = ReportJob::query()->find($this->reportJobId); - if ($job === null) { - Log::warning('GenerateReportJob: report_job not found', ['id' => $this->reportJobId]); + // SET LOCAL inside a transaction establishes the tenant GUC for the + // duration of this block — required by RLS on report_jobs for + // crm_app_user (non-BYPASSRLS) on production. + DB::transaction(function () use ($registry): void { + DB::statement('SET LOCAL app.current_tenant_id = '.$this->tenantId); - return; - } - - if (! in_array($job->status, ReportJob::ACTIVE_STATUSES, true)) { - // Уже terminal — повторный dispatch (например, Horizon retry) пропускаем. - return; - } - - $job->update(['status' => ReportJob::STATUS_PROCESSING]); - - $startedAt = microtime(true); - try { - $params = $job->parameters ?? []; - $format = (string) ($params['format'] ?? 'csv'); - - if (! $registry->isSupported($job->type, $format)) { - $this->markFailed($job, "Неподдерживаемая комбинация: {$job->type}/{$format}", $startedAt); + $job = ReportJob::query()->find($this->reportJobId); + if ($job === null) { + Log::warning('GenerateReportJob: report_job not found', ['id' => $this->reportJobId]); return; } - $provider = $registry->provider($job->type); - $formatter = $registry->formatter($format); + if (! in_array($job->status, ReportJob::ACTIVE_STATUSES, true)) { + // Уже terminal — повторный dispatch (например, Horizon retry) пропускаем. + return; + } - $headers = $provider->headers(); - $rows = $provider->rows($job); - $content = $formatter->format($headers, $rows); + $job->update(['status' => ReportJob::STATUS_PROCESSING]); - $relativePath = sprintf( - 'reports/%d/%d.%s', - $job->tenant_id, - $job->id, - $formatter->fileExtension() - ); - Storage::disk('local')->put($relativePath, $content); + $startedAt = microtime(true); + try { + $params = $job->parameters ?? []; + $format = (string) ($params['format'] ?? 'csv'); - $job->update([ - 'status' => ReportJob::STATUS_DONE, - 'file_path' => $relativePath, - 'file_size' => strlen($content), - 'generation_seconds' => max(1, (int) round(microtime(true) - $startedAt)), - 'finished_at' => Carbon::now(), - 'expires_at' => Carbon::now()->addDays(30), - ]); - } catch (Throwable $e) { - $this->markFailed($job, mb_substr($e->getMessage(), 0, 1000), $startedAt); - Log::error('GenerateReportJob failed', [ - 'id' => $this->reportJobId, - 'exception' => $e, - ]); - } + if (! $registry->isSupported($job->type, $format)) { + $this->markFailed($job, "Неподдерживаемая комбинация: {$job->type}/{$format}", $startedAt); + + return; + } + + $provider = $registry->provider($job->type); + $formatter = $registry->formatter($format); + + $headers = $provider->headers(); + $rows = $provider->rows($job); + $content = $formatter->format($headers, $rows); + + $relativePath = sprintf( + 'reports/%d/%d.%s', + $job->tenant_id, + $job->id, + $formatter->fileExtension() + ); + Storage::disk('local')->put($relativePath, $content); + + $job->update([ + 'status' => ReportJob::STATUS_DONE, + 'file_path' => $relativePath, + 'file_size' => strlen($content), + 'generation_seconds' => max(1, (int) round(microtime(true) - $startedAt)), + 'finished_at' => Carbon::now(), + 'expires_at' => Carbon::now()->addDays(30), + ]); + } catch (Throwable $e) { + $this->markFailed($job, mb_substr($e->getMessage(), 0, 1000), $startedAt); + Log::error('GenerateReportJob failed', [ + 'id' => $this->reportJobId, + 'exception' => $e, + ]); + } + }); } private function markFailed(ReportJob $job, string $message, float $startedAt): void diff --git a/app/app/Jobs/ProcessWebhookJob.php b/app/app/Jobs/ProcessWebhookJob.php index 45f931ca..a7fd198f 100644 --- a/app/app/Jobs/ProcessWebhookJob.php +++ b/app/app/Jobs/ProcessWebhookJob.php @@ -370,7 +370,11 @@ class ProcessWebhookJob implements ShouldQueue */ public function failed(Throwable $e): void { - DB::table('failed_webhook_jobs')->insert([ + // failed_webhook_jobs is an RLS-protected table. On production crm_app_user + // (non-BYPASSRLS) there is no app.current_tenant_id GUC in the failed() + // callback context. Use pgsql_supplier (crm_supplier_worker, BYPASSRLS) — + // same pattern as RouteSupplierLeadJob::failed(). + DB::connection('pgsql_supplier')->table('failed_webhook_jobs')->insert([ 'tenant_id' => $this->tenantId, 'webhook_log_id' => $this->webhookLogId, 'raw_payload' => json_encode($this->data, JSON_UNESCAPED_UNICODE), diff --git a/app/tests/Feature/ProcessWebhookJobTest.php b/app/tests/Feature/ProcessWebhookJobTest.php index 726dc35a..73f24dbf 100644 --- a/app/tests/Feature/ProcessWebhookJobTest.php +++ b/app/tests/Feature/ProcessWebhookJobTest.php @@ -14,6 +14,7 @@ use App\Models\WebhookDedupKey; use Illuminate\Foundation\Testing\DatabaseTransactions; use Illuminate\Support\Facades\DB; use Illuminate\Support\Str; +use Tests\Concerns\SharesSupplierPdo; /** * Тесты ProcessWebhookJob — двустадийный dedup v8.6 (CTO-17). @@ -25,8 +26,12 @@ use Illuminate\Support\Str; * NB: Job::handle() сам открывает DB::transaction. DatabaseTransactions * trait оборачивает каждый тест в outer-транзакцию — Laravel-PG-driver * корректно обрабатывает nested через savepoints. + * + * SharesSupplierPdo: failed() now inserts via pgsql_supplier (BYPASSRLS) — + * share PDO so DatabaseTransactions cross-connection visibility works on dev. */ uses(DatabaseTransactions::class); +uses(SharesSupplierPdo::class); function makePayload(int $vid = 432176649, ?int $time = null): array { diff --git a/app/tests/Feature/RemindersDispatchDueTest.php b/app/tests/Feature/RemindersDispatchDueTest.php index ddca06f9..d2302546 100644 --- a/app/tests/Feature/RemindersDispatchDueTest.php +++ b/app/tests/Feature/RemindersDispatchDueTest.php @@ -10,8 +10,10 @@ use App\Models\User; use Illuminate\Foundation\Testing\DatabaseTransactions; use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Mail; +use Tests\Concerns\SharesSupplierPdo; uses(DatabaseTransactions::class); +uses(SharesSupplierPdo::class); beforeEach(function () { Mail::fake(); diff --git a/app/tests/Feature/Reports/ReportLifecycleTest.php b/app/tests/Feature/Reports/ReportLifecycleTest.php index 02b51180..895d669d 100644 --- a/app/tests/Feature/Reports/ReportLifecycleTest.php +++ b/app/tests/Feature/Reports/ReportLifecycleTest.php @@ -11,8 +11,10 @@ use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Bus; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Storage; +use Tests\Concerns\SharesSupplierPdo; uses(DatabaseTransactions::class); +uses(SharesSupplierPdo::class); beforeEach(function () { $this->tenant = Tenant::factory()->create();