RomanNum3ral
/
whisper-money
mirror of https://github.com/whisper-money/whisper-money.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(security): scope job-status cache keys to the owning user 

The categorization, bulk rule re-evaluation, and automation-rule apply
status endpoints looked jobs up by a bare job UUID. Any authenticated
user who obtained another user's job id could poll its progress payload.

Include the owning user's id in the cache key so a status request keyed
by the polling user's id can only resolve that user's own jobs; a
mismatched owner now falls through to the existing 404. No ownership
store or extra lookup needed. Added cross-user isolation tests for all
three endpoints.
This commit is contained in:
Víctor Falcón 2026-07-03 13:09:18 +02:00
parent eb31455e60
commit 93e2d6823c
10 changed files with 75 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/Actions/Ai/StartCategorizationBackfill.php
View File

@ -37,7 +37,7 @@ class StartCategorizationBackfill
$jobId = (string) Str::uuid();
Cache::put(
CategorizeUncategorizedTransactionsJob::cacheKeyForJobId($jobId),
CategorizeUncategorizedTransactionsJob::cacheKeyForJobId($user->id, $jobId),
['status' => 'pending', 'processed' => 0, 'total' => $total, 'applied' => 0],
now()->addHour(),
);

5
app/Http/Controllers/Ai/CategorizationController.php
View File

@ -5,6 +5,7 @@ namespace App\Http\Controllers\Ai;
use App\Http\Controllers\Controller;
use App\Jobs\CategorizeUncategorizedTransactionsJob;
use Illuminate\Http\JsonResponse;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\Cache;
class CategorizationController extends Controller
@ -12,9 +13,9 @@ class CategorizationController extends Controller
/**
* Return current progress for a consent-triggered categorization backfill.
*/
public function status(string $jobId): JsonResponse
public function status(Request $request, string $jobId): JsonResponse
{
$progress = Cache::get(CategorizeUncategorizedTransactionsJob::cacheKeyForJobId($jobId));
$progress = Cache::get(CategorizeUncategorizedTransactionsJob::cacheKeyForJobId($request->user()->id, $jobId));
if ($progress === null) {
return response()->json(['message' => 'Job not found.'], 404);

4
app/Http/Controllers/ReEvaluateTransactionRulesController.php
View File

@ -47,7 +47,7 @@ class ReEvaluateTransactionRulesController extends Controller
// Set initial pending state so the first poll returns something meaningful
Cache::put(
ReEvaluateTransactionRulesJob::cacheKeyForJobId($jobId),
ReEvaluateTransactionRulesJob::cacheKeyForJobId($user->id, $jobId),
['status' => 'pending', 'processed' => 0, 'total' => 0, 'updated' => 0],
now()->addHour(),
);
@ -64,7 +64,7 @@ class ReEvaluateTransactionRulesController extends Controller
*/
public function status(Request $request, string $jobId): JsonResponse
{
$cacheKey = ReEvaluateTransactionRulesJob::cacheKeyForJobId($jobId);
$cacheKey = ReEvaluateTransactionRulesJob::cacheKeyForJobId($request->user()->id, $jobId);
$progress = Cache::get($cacheKey);
if ($progress === null) {

4
app/Http/Controllers/Settings/AutomationRuleApplicationController.php
View File

@ -121,7 +121,7 @@ class AutomationRuleApplicationController extends Controller
$jobId = (string) Str::uuid();
Cache::put(
ApplySingleAutomationRuleJob::cacheKeyForJobId($jobId),
ApplySingleAutomationRuleJob::cacheKeyForJobId($automationRule->user_id, $jobId),
['status' => 'pending', 'processed' => 0, 'total' => $total, 'applied' => 0, 'updated' => 0],
now()->addHour(),
);
@ -141,7 +141,7 @@ class AutomationRuleApplicationController extends Controller
*/
public function status(Request $request, string $jobId): JsonResponse
{
$progress = Cache::get(ApplySingleAutomationRuleJob::cacheKeyForJobId($jobId));
$progress = Cache::get(ApplySingleAutomationRuleJob::cacheKeyForJobId($request->user()->id, $jobId));
if ($progress === null) {
return response()->json(['message' => 'Job not found.'], 404);

6
app/Jobs/ApplySingleAutomationRuleJob.php
View File

@ -69,14 +69,14 @@ class ApplySingleAutomationRuleJob implements ShouldQueue
);
}
public static function cacheKeyForJobId(string $jobId): string
public static function cacheKeyForJobId(string $userId, string $jobId): string
{
return "apply_automation_rule_job_{$jobId}";
return "apply_automation_rule_job_{$userId}_{$jobId}";
}
private function cacheKey(): string
{
return self::cacheKeyForJobId($this->jobId);
return self::cacheKeyForJobId($this->rule->user_id, $this->jobId);
}
/**

8
app/Jobs/CategorizeUncategorizedTransactionsJob.php
View File

@ -41,9 +41,9 @@ class CategorizeUncategorizedTransactionsJob implements ShouldQueue
return (string) config('ai_categorization.queue');
}
public static function cacheKeyForJobId(string $jobId): string
public static function cacheKeyForJobId(string $userId, string $jobId): string
{
return "categorize_transactions_job_{$jobId}";
return "categorize_transactions_job_{$userId}_{$jobId}";
}
public function handle(AiCategorizationGate $gate, AiCategorizer $categorizer): void
@ -68,7 +68,7 @@ class CategorizeUncategorizedTransactionsJob implements ShouldQueue
*/
public function failed(?Throwable $exception): void
{
$progress = Cache::get(self::cacheKeyForJobId($this->jobId), [
$progress = Cache::get(self::cacheKeyForJobId($this->user->id, $this->jobId), [
'processed' => 0,
'total' => 0,
'applied' => 0,
@ -87,7 +87,7 @@ class CategorizeUncategorizedTransactionsJob implements ShouldQueue
*/
private function updateProgress(string $status, int $processed, int $total, int $applied): void
{
Cache::put(self::cacheKeyForJobId($this->jobId), [
Cache::put(self::cacheKeyForJobId($this->user->id, $this->jobId), [
'status' => $status,
'processed' => $processed,
'total' => $total,

6
app/Jobs/ReEvaluateTransactionRulesJob.php
View File

@ -83,14 +83,14 @@ class ReEvaluateTransactionRulesJob implements ShouldQueue
);
}
public static function cacheKeyForJobId(string $jobId): string
public static function cacheKeyForJobId(string $userId, string $jobId): string
{
return "re_evaluate_rules_job_{$jobId}";
return "re_evaluate_rules_job_{$userId}_{$jobId}";
}
private function cacheKey(): string
{
return self::cacheKeyForJobId($this->jobId);
return self::cacheKeyForJobId($this->user->id, $this->jobId);
}
/**

22
tests/Feature/Ai/CategorizeUncategorizedTransactionsTest.php
View File

@ -85,7 +85,7 @@ it('does not dispatch a backfill when AI categorization is disabled', function (
it('returns categorization progress from the status endpoint', function () {
$user = User::factory()->create();
Cache::put(
CategorizeUncategorizedTransactionsJob::cacheKeyForJobId('job-123'),
CategorizeUncategorizedTransactionsJob::cacheKeyForJobId($user->id, 'job-123'),
['status' => 'processing', 'processed' => 1, 'total' => 4, 'applied' => 1],
now()->addHour(),
);
@ -102,6 +102,20 @@ it('returns 404 from the status endpoint for an unknown job', function () {
->assertNotFound();
});
it('does not leak another user\'s categorization progress', function () {
$owner = User::factory()->create();
Cache::put(
CategorizeUncategorizedTransactionsJob::cacheKeyForJobId($owner->id, 'job-123'),
['status' => 'processing', 'processed' => 1, 'total' => 4, 'applied' => 1],
now()->addHour(),
);
$otherUser = User::factory()->create();
actingAs($otherUser)->getJson(route('ai.categorization.status', 'job-123'))
->assertNotFound();
});
it('records progress while categorizing the uncategorized transactions', function () {
$user = User::factory()->create();
$user->recordAiConsent();
@ -139,7 +153,7 @@ it('records progress while categorizing the uncategorized transactions', functio
$jobId = 'job-run-1';
app()->call([new CategorizeUncategorizedTransactionsJob($user, $jobId), 'handle']);
$progress = Cache::get(CategorizeUncategorizedTransactionsJob::cacheKeyForJobId($jobId));
$progress = Cache::get(CategorizeUncategorizedTransactionsJob::cacheKeyForJobId($user->id, $jobId));
expect($progress['status'])->toBe('done')
->and($progress['total'])->toBe(2)
@ -151,7 +165,7 @@ it('marks the cache as failed and preserves counts when the job fails', function
$user = User::factory()->create();
$jobId = 'failed-job';
Cache::put(
CategorizeUncategorizedTransactionsJob::cacheKeyForJobId($jobId),
CategorizeUncategorizedTransactionsJob::cacheKeyForJobId($user->id, $jobId),
['status' => 'processing', 'processed' => 3, 'total' => 10, 'applied' => 2],
now()->addHour(),
);
@ -159,7 +173,7 @@ it('marks the cache as failed and preserves counts when the job fails', function
(new CategorizeUncategorizedTransactionsJob($user, $jobId))
->failed(new RuntimeException('boom'));
$progress = Cache::get(CategorizeUncategorizedTransactionsJob::cacheKeyForJobId($jobId));
$progress = Cache::get(CategorizeUncategorizedTransactionsJob::cacheKeyForJobId($user->id, $jobId));
expect($progress['status'])->toBe('failed')
->and($progress['processed'])->toBe(3)

20
tests/Feature/AutomationRuleApplicationTest.php
View File

@ -318,6 +318,26 @@ test('cannot apply rule belonging to another user', function () {
->assertForbidden();
});
test('cannot poll apply job status belonging to another user', function () {
$jobId = 'apply-job-1';
Cache::put(
ApplySingleAutomationRuleJob::cacheKeyForJobId($this->user->id, $jobId),
['status' => 'processing', 'processed' => 1, 'total' => 4],
now()->addHour(),
);
$this->actingAs($this->user)
->getJson(route('automation-rules.apply.status', $jobId))
->assertOk()
->assertJsonPath('status', 'processing');
$otherUser = User::factory()->onboarded()->create();
$this->actingAs($otherUser)
->getJson(route('automation-rules.apply.status', $jobId))
->assertNotFound();
});
test('label-only rule applies when only_uncategorized is true', function () {
$labelOnlyRule = AutomationRule::factory()->create([
'user_id' => $this->user->id,

25
tests/Feature/ReEvaluateTransactionRulesTest.php
View File

@ -213,7 +213,7 @@ test('bulk endpoint sets initial pending status in cache', function () {
->postJson(route('transactions.re-evaluate-rules.bulk'));
$jobId = $response->json('job_id');
$cacheKey = ReEvaluateTransactionRulesJob::cacheKeyForJobId($jobId);
$cacheKey = ReEvaluateTransactionRulesJob::cacheKeyForJobId($this->user->id, $jobId);
expect(Cache::get($cacheKey))->toMatchArray(['status' => 'pending']);
});
@ -224,7 +224,7 @@ test('bulk endpoint sets initial pending status in cache', function () {
test('status endpoint returns progress from cache', function () {
$jobId = 'test-job-id';
$cacheKey = ReEvaluateTransactionRulesJob::cacheKeyForJobId($jobId);
$cacheKey = ReEvaluateTransactionRulesJob::cacheKeyForJobId($this->user->id, $jobId);
Cache::put($cacheKey, [
'status' => 'processing',
@ -250,6 +250,19 @@ test('status endpoint returns 404 for unknown job', function () {
->assertNotFound();
});
test('status endpoint does not leak another user\'s job progress', function () {
$jobId = 'owned-by-someone-else';
$cacheKey = ReEvaluateTransactionRulesJob::cacheKeyForJobId($this->user->id, $jobId);
Cache::put($cacheKey, ['status' => 'processing', 'processed' => 5, 'total' => 50, 'updated' => 1], now()->addHour());
$otherUser = User::factory()->onboarded()->create();
$this->actingAs($otherUser)
->getJson(route('transactions.re-evaluate-rules.status', $jobId))
->assertNotFound();
});
// ──────────────────────────────────────────────
// Job execution
// ──────────────────────────────────────────────
@ -287,7 +300,7 @@ test('job applies rules to non-encrypted transactions and tracks progress', func
expect($matchingTransaction->fresh()->category_id)->toBe($this->category->id);
expect($nonMatchingTransaction->fresh()->category_id)->toBeNull();
$progress = Cache::get(ReEvaluateTransactionRulesJob::cacheKeyForJobId($jobId));
$progress = Cache::get(ReEvaluateTransactionRulesJob::cacheKeyForJobId($this->user->id, $jobId));
expect($progress['status'])->toBe('done');
expect($progress['processed'])->toBe(2);
expect($progress['updated'])->toBe(1);
@ -314,7 +327,7 @@ test('job skips encrypted transactions', function () {
$job = new ReEvaluateTransactionRulesJob($this->user, $jobId);
$job->handle(app(AutomationRuleService::class));
$progress = Cache::get(ReEvaluateTransactionRulesJob::cacheKeyForJobId($jobId));
$progress = Cache::get(ReEvaluateTransactionRulesJob::cacheKeyForJobId($this->user->id, $jobId));
// 0 processed because the encrypted transaction is excluded from the query
expect($progress['processed'])->toBe(0);
expect($progress['updated'])->toBe(0);
@ -352,7 +365,7 @@ test('job only processes provided transaction_ids', function () {
expect($t1->fresh()->category_id)->toBe($this->category->id);
expect($t2->fresh()->category_id)->toBeNull();
$progress = Cache::get(ReEvaluateTransactionRulesJob::cacheKeyForJobId($jobId));
$progress = Cache::get(ReEvaluateTransactionRulesJob::cacheKeyForJobId($this->user->id, $jobId));
expect($progress['processed'])->toBe(1);
});
@ -412,7 +425,7 @@ test('job applies rules to transactions matching filters', function () {
expect($matchingTransaction->fresh()->category_id)->toBe($this->category->id);
expect($outsideRangeTransaction->fresh()->category_id)->toBeNull();
$progress = Cache::get(ReEvaluateTransactionRulesJob::cacheKeyForJobId($jobId));
$progress = Cache::get(ReEvaluateTransactionRulesJob::cacheKeyForJobId($this->user->id, $jobId));
expect($progress['status'])->toBe('done');
expect($progress['processed'])->toBe(1);
expect($progress['updated'])->toBe(1);