Skip to content

Commit c27da1f

Browse files
freekmurzefreekmurze
authored
Store attachments in dedicated column instead of overwriting payload (#249)
* Store attachments in dedicated column instead of overwriting payload Fixes #248. PR #235 stored uploaded files under the 'attachments' key of the payload, which silently overwrote that key for any webhook whose JSON body legitimately uses 'attachments'. Files now go to a dedicated nullable JSON 'attachments' column on the webhook_calls table. The payload is no longer touched. Backwards compatibility: - getAttachments() reads from the new column and falls back to payload['attachments'] for rows written by 3.5.x, so historical data stays accessible without manual migration. - A new add_attachments_to_webhook_calls_table migration is registered for existing installs. - A per-config 'store_attachments' option (default true) lets users opt out of file storage. The only code-side break is direct reads of $call->payload['attachments'] on new rows; those callers should switch to $call->getAttachments(), which has been the documented accessor since #235. * Add larastan to require-dev so PHPStan can load checkOctaneCompatibility and checkModelProperties The phpstan.neon.dist has referenced these larastan-specific config keys since at least July 2025, but larastan was never declared as a dev dep. PHPStan has been failing on every CI run as a result. Adds larastan ^2.9 which matches the existing PHPStan 1.12 line. * Revert "Add larastan to require-dev so PHPStan can load checkOctaneCompatibility and checkModelProperties" This reverts commit 610b667. * Drop larastan-specific keys from phpstan.neon.dist checkOctaneCompatibility and checkModelProperties require larastan, which can't be added to require-dev without restricting Laravel versions in the test matrix (larastan 2.x doesn't support L12/L13 and larastan 3.x requires PHPStan 2.x). Removing these keys lets PHPStan run cleanly with the level 5 base ruleset. * Ignore Eloquent magic-method PHPStan errors that previously needed larastan Without larastan, PHPStan does not understand Laravel's Eloquent magic statics like Model::create() and Model::where(), nor the IDE-helper \Eloquent mixin. Adds targeted ignoreErrors entries for the specific patterns used in WebhookCall, so PHPStan runs cleanly without pulling larastan into require-dev (which would conflict with Laravel 12/13 in the test matrix).
1 parent ed0550f commit c27da1f

9 files changed

Lines changed: 198 additions & 32 deletions

‎config/webhook-client.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,13 @@
5353

5454
],
5555

56+
/*
57+
* When set to true, file uploads on the incoming request are stored
58+
* in the dedicated `attachments` column on the webhook call model.
59+
* Set to false to skip file extraction entirely.
60+
*/
61+
'store_attachments' => true,
62+
5663
/*
5764
* The class name of the job that will process the webhook request.
5865
*

‎database/migrations/add_attachments_to_webhook_calls_table.php.stub‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
3+
use Illuminate\Database\Schema\Blueprint;
4+
use Illuminate\Database\Migrations\Migration;
5+
use Illuminate\Support\Facades\Schema;
6+
7+
return new class extends Migration
8+
{
9+
public function up()
10+
{
11+
if (Schema::hasColumn('webhook_calls', 'attachments')) {
12+
return;
13+
}
14+
15+
Schema::table('webhook_calls', function (Blueprint $table) {
16+
$table->json('attachments')->nullable()->after('payload');
17+
});
18+
}
19+
};

‎database/migrations/create_webhook_calls_table.php.stub‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ return new class extends Migration
1515
$table->string('url', 512);
1616
$table->json('headers')->nullable();
1717
$table->json('payload')->nullable();
18+
$table->json('attachments')->nullable();
1819
$table->text('exception')->nullable();
1920

2021
$table->timestamps();

‎phpstan.neon.dist‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ parameters:
88
- config
99
- database
1010
tmpDir: build/phpstan
11-
checkOctaneCompatibility: true
12-
checkModelProperties: true
1311
checkMissingIterableValueType: false
1412

1513
ignoreErrors:
1614
- '#Unsafe usage of new static#'
15+
- '#PHPDoc tag @mixin contains unknown class Eloquent#'
16+
- '#Call to an undefined static method Spatie\\WebhookClient\\Models\\WebhookCall::(create|where)\(\)#'

‎src/Models/WebhookCall.php‎

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
* @property string $url
2121
* @property array|null $headers
2222
* @property array|null $payload
23+
* @property array|null $attachments
2324
* @property array|null $exception
2425
* @property \Illuminate\Support\Carbon|null $created_at
2526
* @property \Illuminate\Support\Carbon|null $updated_at
@@ -43,32 +44,40 @@ class WebhookCall extends Model
4344
protected $casts = [
4445
'headers' => 'array',
4546
'payload' => 'array',
47+
'attachments' => 'array',
4648
'exception' => 'array',
4749
];
4850

4951
public static function storeWebhook(WebhookConfig $config, Request $request): WebhookCall
5052
{
51-
$headers = self::headersToStore($config, $request);
52-
$payload = self::buildPayloadFromRequest($request);
53-
5453
return self::create([
5554
'name' => $config->name,
5655
'url' => $request->fullUrl(),
57-
'headers' => $headers,
58-
'payload' => $payload,
56+
'headers' => self::headersToStore($config, $request),
57+
'payload' => self::buildPayloadFromRequest($request),
58+
'attachments' => self::buildAttachmentsFromRequest($config, $request),
5959
'exception' => null,
6060
]);
6161
}
6262

6363
protected static function buildPayloadFromRequest(Request $request): array
6464
{
65-
$payload = $request->input();
65+
return $request->input();
66+
}
6667

67-
if ($request->allFiles()) {
68-
$payload['attachments'] = self::processRequestFiles($request->allFiles());
68+
protected static function buildAttachmentsFromRequest(WebhookConfig $config, Request $request): ?array
69+
{
70+
if (! $config->storeAttachments) {
71+
return null;
6972
}
7073

71-
return $payload;
74+
$files = $request->allFiles();
75+
76+
if (empty($files)) {
77+
return null;
78+
}
79+
80+
return self::processRequestFiles($files);
7281
}
7382

7483
protected static function processRequestFiles(array $files): array
@@ -157,20 +166,20 @@ public function prunable()
157166
}
158167

159168
/**
160-
* Convert stored file metadata back into UploadedFile objects
169+
* Convert stored file metadata back into UploadedFile objects.
170+
*
171+
* Reads from the dedicated attachments column and falls back to
172+
* `payload['attachments']` for rows written by older versions of the
173+
* package that stored attachments inside the payload.
161174
*
162175
* @return array
163176
*/
164177
public function getAttachments(): array
165178
{
166-
if (! isset($this->payload['attachments'])) {
167-
return [];
168-
}
179+
$attachments = $this->attachments ?? $this->payload['attachments'] ?? [];
169180

170-
return collect($this->payload['attachments'])
171-
->map(function ($attachment) {
172-
return $this->createUploadedFileFromAttachment($attachment);
173-
})
181+
return collect($attachments)
182+
->map(fn ($attachment) => $this->createUploadedFileFromAttachment($attachment))
174183
->toArray();
175184
}
176185

‎src/WebhookClientServiceProvider.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ public function configurePackage(Package $package): void
1616
$package
1717
->name('laravel-webhook-client')
1818
->hasConfigFile()
19-
->hasMigrations('create_webhook_calls_table');
19+
->hasMigrations(
20+
'create_webhook_calls_table',
21+
'add_attachments_to_webhook_calls_table',
22+
);
2023
}
2124

2225
public function packageRegistered()

‎src/WebhookConfig.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ class WebhookConfig
2727

2828
public array | string $storeHeaders;
2929

30+
public bool $storeAttachments;
31+
3032
public string $processWebhookJobClass;
3133

3234
public function __construct(array $properties)
@@ -57,6 +59,8 @@ public function __construct(array $properties)
5759

5860
$this->storeHeaders = $properties['store_headers'] ?? [];
5961

62+
$this->storeAttachments = $properties['store_attachments'] ?? true;
63+
6064
if (! is_subclass_of($properties['process_webhook_job'], ProcessWebhookJob::class)) {
6165
throw InvalidConfig::invalidProcessWebhookJob($properties['process_webhook_job']);
6266
}

‎tests/WebhookCallModelTest.php‎

Lines changed: 117 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
expect($webhookCall)->toBeInstanceOf(WebhookCall::class);
2929
expect($webhookCall->name)->toBe('test');
3030
expect($webhookCall->payload)->toBe(['key' => 'value']);
31-
expect($webhookCall->payload)->not->toHaveKey('attachments');
31+
expect($webhookCall->attachments)->toBeNull();
3232
});
3333

3434
it('can store webhook with single file', function () {
@@ -43,11 +43,11 @@
4343

4444
expect($webhookCall)->toBeInstanceOf(WebhookCall::class);
4545
expect($webhookCall->name)->toBe('test');
46-
expect($webhookCall->payload['key'])->toBe('value');
47-
expect($webhookCall->payload)->toHaveKey('attachments');
48-
expect($webhookCall->payload['attachments'])->toHaveCount(1);
46+
expect($webhookCall->payload)->toBe(['key' => 'value']);
47+
expect($webhookCall->payload)->not->toHaveKey('attachments');
48+
expect($webhookCall->attachments)->toHaveCount(1);
4949

50-
$attachment = $webhookCall->payload['attachments'][0];
50+
$attachment = $webhookCall->attachments[0];
5151
expect($attachment['originalName'])->toBe('test.txt');
5252
expect($attachment['mimeType'])->not->toBeEmpty();
5353
expect($attachment['size'])->toBeGreaterThan(0);
@@ -66,14 +66,13 @@
6666
$webhookCall = WebhookCall::storeWebhook($this->webhookConfig, $request);
6767

6868
expect($webhookCall)->toBeInstanceOf(WebhookCall::class);
69-
expect($webhookCall->payload)->toHaveKey('attachments');
70-
expect($webhookCall->payload['attachments'])->toHaveCount(2);
69+
expect($webhookCall->attachments)->toHaveCount(2);
7170

72-
$attachment1 = $webhookCall->payload['attachments'][0];
71+
$attachment1 = $webhookCall->attachments[0];
7372
expect($attachment1['originalName'])->toBe('test1.txt');
7473
expect($attachment1['size'])->toBeGreaterThan(0);
7574

76-
$attachment2 = $webhookCall->payload['attachments'][1];
75+
$attachment2 = $webhookCall->attachments[1];
7776
expect($attachment2['originalName'])->toBe('test2.pdf');
7877
expect($attachment2['size'])->toBeGreaterThan(0);
7978
});
@@ -92,15 +91,69 @@
9291
$webhookCall = WebhookCall::storeWebhook($this->webhookConfig, $request);
9392

9493
expect($webhookCall)->toBeInstanceOf(WebhookCall::class);
95-
expect($webhookCall->payload)->toHaveKey('attachments');
96-
expect($webhookCall->payload['attachments'])->toHaveCount(3);
94+
expect($webhookCall->attachments)->toHaveCount(3);
9795

98-
$fileNames = collect($webhookCall->payload['attachments'])->pluck('originalName')->toArray();
96+
$fileNames = collect($webhookCall->attachments)->pluck('originalName')->toArray();
9997
expect($fileNames)->toContain('single.txt');
10098
expect($fileNames)->toContain('multi1.txt');
10199
expect($fileNames)->toContain('multi2.txt');
102100
});
103101

102+
it('does not overwrite a user-provided attachments key in the payload', function () {
103+
$request = Request::create('/test', 'POST', [
104+
'key' => 'value',
105+
'attachments' => ['user-provided', 'data'],
106+
]);
107+
108+
$webhookCall = WebhookCall::storeWebhook($this->webhookConfig, $request);
109+
110+
expect($webhookCall->payload['attachments'])->toBe(['user-provided', 'data']);
111+
expect($webhookCall->attachments)->toBeNull();
112+
});
113+
114+
it('preserves a user-provided attachments key when files are also uploaded', function () {
115+
Storage::fake('local');
116+
117+
$file = UploadedFile::fake()->create('test.txt', 1);
118+
119+
$request = Request::create('/test', 'POST', [
120+
'attachments' => ['user-provided'],
121+
]);
122+
$request->files->set('document', $file);
123+
124+
$webhookCall = WebhookCall::storeWebhook($this->webhookConfig, $request);
125+
126+
expect($webhookCall->payload['attachments'])->toBe(['user-provided']);
127+
expect($webhookCall->attachments)->toHaveCount(1);
128+
expect($webhookCall->attachments[0]['originalName'])->toBe('test.txt');
129+
});
130+
131+
it('does not extract files when store_attachments is disabled', function () {
132+
Storage::fake('local');
133+
134+
$config = new WebhookConfig([
135+
'name' => 'test',
136+
'signing_secret' => 'secret',
137+
'signature_header_name' => 'Signature',
138+
'signature_validator' => \Spatie\WebhookClient\SignatureValidator\DefaultSignatureValidator::class,
139+
'webhook_profile' => \Spatie\WebhookClient\WebhookProfile\ProcessEverythingWebhookProfile::class,
140+
'webhook_response' => \Spatie\WebhookClient\WebhookResponse\DefaultRespondsTo::class,
141+
'webhook_model' => WebhookCall::class,
142+
'process_webhook_job' => \Spatie\WebhookClient\Tests\TestClasses\ProcessWebhookJobTestClass::class,
143+
'store_headers' => [],
144+
'store_attachments' => false,
145+
]);
146+
147+
$file = UploadedFile::fake()->create('test.txt', 1);
148+
$request = Request::create('/test', 'POST', ['key' => 'value']);
149+
$request->files->set('document', $file);
150+
151+
$webhookCall = WebhookCall::storeWebhook($config, $request);
152+
153+
expect($webhookCall->attachments)->toBeNull();
154+
expect($webhookCall->payload)->not->toHaveKey('attachments');
155+
});
156+
104157
it('can retrieve attachments as uploaded file objects', function () {
105158
Storage::fake('local');
106159

@@ -128,6 +181,58 @@
128181
expect($attachments)->toBeEmpty();
129182
});
130183

184+
it('falls back to payload attachments for rows written by older versions', function () {
185+
$legacyAttachment = [
186+
'originalName' => 'legacy.txt',
187+
'mimeType' => 'text/plain',
188+
'size' => 7,
189+
'error' => 0,
190+
'path' => '/tmp/legacy',
191+
'content' => base64_encode('legacy!'),
192+
];
193+
194+
$webhookCall = WebhookCall::create([
195+
'name' => 'test',
196+
'url' => 'http://example.test/webhook',
197+
'headers' => [],
198+
'payload' => ['key' => 'value', 'attachments' => [$legacyAttachment]],
199+
'attachments' => null,
200+
'exception' => null,
201+
]);
202+
203+
$attachments = $webhookCall->getAttachments();
204+
205+
expect($attachments)->toHaveCount(1);
206+
expect($attachments[0])->toBeInstanceOf(UploadedFile::class);
207+
expect($attachments[0]->getClientOriginalName())->toBe('legacy.txt');
208+
expect(file_get_contents($attachments[0]->getPathname()))->toBe('legacy!');
209+
});
210+
211+
it('prefers the attachments column over a payload attachments key when both exist', function () {
212+
$columnAttachment = [
213+
'originalName' => 'column.txt',
214+
'mimeType' => 'text/plain',
215+
'size' => 6,
216+
'error' => 0,
217+
'path' => '/tmp/column',
218+
'content' => base64_encode('column'),
219+
];
220+
221+
$webhookCall = WebhookCall::create([
222+
'name' => 'test',
223+
'url' => 'http://example.test/webhook',
224+
'headers' => [],
225+
'payload' => ['attachments' => ['user-provided']],
226+
'attachments' => [$columnAttachment],
227+
'exception' => null,
228+
]);
229+
230+
$attachments = $webhookCall->getAttachments();
231+
232+
expect($attachments)->toHaveCount(1);
233+
expect($attachments[0]->getClientOriginalName())->toBe('column.txt');
234+
});
235+
131236
it('can retrieve multiple attachments as uploaded file objects', function () {
132237
Storage::fake('local');
133238

‎tests/WebhookConfigTest.php‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,24 @@
5757
expect(fn () => new WebhookConfig($config))->toThrow(InvalidConfig::class);
5858
});
5959

60+
it('defaults storeAttachments to true when not provided', function () {
61+
$config = getValidConfig();
62+
unset($config['store_attachments']);
63+
64+
$webhookConfig = new WebhookConfig($config);
65+
66+
expect($webhookConfig->storeAttachments)->toBeTrue();
67+
});
68+
69+
it('respects an explicit storeAttachments value', function () {
70+
$config = getValidConfig();
71+
$config['store_attachments'] = false;
72+
73+
$webhookConfig = new WebhookConfig($config);
74+
75+
expect($webhookConfig->storeAttachments)->toBeFalse();
76+
});
77+
6078
function getValidConfig(): array
6179
{
6280
return [

0 commit comments

Comments
 (0)