Sync main repo#3
Conversation
Use strlen($pixel) instead of hardcoded 42 to ensure Content-Length matches the actual response size. Prevents FastCGI upstream warnings and ensures proper response behaviour across environments.
This PR declares the property `sent_email` in the `ValidActionEvent` class.
WalkthroughMeu caro amigo! 🎉 Esse PR traz uma refatoração bem robusta no rastreamento de links do mail-tracker. A mudança principal é migrar de URLs simples para URLs assinadas com validação de domínio, remove o antigo helper ChangesLink Validation com Signed Routes & Test Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutos Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.1.54)Composer install failed: this project depends on private packages that require authentication (e.g. GitLab/GitHub, Laravel Nova, etc.). Instead, run PHPStan in a CI/CD pipeline where you can use custom packages — our pipeline remediation tool can use the PHPStan output from your CI/CD pipeline. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
src/MailTracker.php (1)
218-224: Assinar link com TTL e escapar HTML no href.
- Segurança: considere expirar a assinatura (ex.: horas/dias) para reduzir replay. Sugiro TTL configurável.
- Robustez: encode do URL no atributo
hrefevita HTML inválido quando houver&/aspas.Aplicar:
- return $matches[1].URL::signedRoute( - 'mailTracker_n', - [ - 'l' => $url, - 'h' => $this->hash, - ] - ); + $ttl = config('mail-tracker.link-signature-ttl'); // segundos; null = sem expiração + $signedUrl = $ttl + ? URL::temporarySignedRoute( + 'mailTracker_n', + now()->addSeconds($ttl), + ['l' => $url, 'h' => $this->hash] + ) + : URL::signedRoute('mailTracker_n', ['l' => $url, 'h' => $this->hash]); + return $matches[1] . e($signedUrl);src/Model/SentEmail.php (1)
57-69: Extrair domínios com mais precisão (suportar aspas simples e ignorar vazios).Evita inserir
nulle cobrehref='...'.- public function getDomainsInContextAttribute(){ - preg_match_all("/(<a[^>]*href=[\"])([^\"]*)/", $this->content, $matches); - if ( ! isset($matches[2] ) ) return []; - $domains = []; - foreach($matches[2] as $url){ - $domain = parse_url($url, PHP_URL_HOST); - if ( ! in_array($domain, $domains) ){ - $domains[] = $domain; - } - } - return $domains; - } + public function getDomainsInContextAttribute() + { + if (! $this->content) return []; + preg_match_all('/<a[^>]*\bhref=(["\'])(.*?)\1/i', $this->content, $matches); + if (! isset($matches[2])) return []; + $set = []; + foreach ($matches[2] as $url) { + $host = parse_url($url, PHP_URL_HOST); + if ($host) { + $set[strtolower($host)] = true; + } + } + return array_keys($set); + }src/Events/ValidActionEvent.php (1)
13-19: Tipar propriedades públicas.Melhora autocompletar e evita uso incorreto.
- public $skip = false; - public $sent_email; + public bool $skip = false; + public Model|SentEmailModel $sent_email;src/Events/ValidLinkEvent.php (1)
13-21: Tipar propriedades e parâmetro$url.Sinaliza contrato claro pro listener.
- public $valid = false; - public $sent_email; - public $url; + public bool $valid = false; + public Model|SentEmailModel $sent_email; + public string $url; - - public function __construct(Model|SentEmailModel $sent_email, $url) + + public function __construct(Model|SentEmailModel $sent_email, string $url)src/Listener/DomainExistsInContentListener.php (1)
11-17: Normalizar host e retornar cedo quando inválido.Evita
in_array(null, ...)e compara case‑insensitive.- $url_host = parse_url($event->url, PHP_URL_HOST); + $url_host = parse_url($event->url, PHP_URL_HOST); + if (! $url_host) { + return; + } + $url_host = strtolower($url_host); // If logging of content is on then if (config('mail-tracker.log-content', true)) { - if ($event->sent_email && !empty($event->sent_email->domains_in_context) && in_array($url_host, $event->sent_email->domains_in_context)) { + $domains = $event->sent_email ? array_map('strtolower', (array) $event->sent_email->domains_in_context) : []; + if (!empty($domains) && in_array($url_host, $domains, true)) { $event->valid = true; } }README.md (2)
192-213: Ajustar indentação dos bullets (MD007).O linter aponta indentação 4 em vez de 2 espaços nos subitens das seções de eventos. Recomendo padronizar para 2 para evitar ruído em CI.
Exemplo de correção (mostrando um bloco representativo):
- - Public attribute `sent_email` contains the `SentEmail` model + - Public attribute `sent_email` contains the `SentEmail` model
211-214: Bullet faltando para TransientBouncedMessageEvent.Adicionar o hífen para manter a lista consistente.
- jdavidbakr\MailTracker\Events\TransientBouncedMessageEvent (when you receive a transient bounce. Check the event's public attributes for `bounce_sub_type` and `diagnostic_code` to determine if you want to do additional processing when this event is received.) +- jdavidbakr\MailTracker\Events\TransientBouncedMessageEvent (when you receive a transient bounce. Check the event's public attributes for `bounce_sub_type` and `diagnostic_code` to determine if you want to do additional processing when this event is received.)config/mail-tracker.php (1)
108-114: Adicionar TTL configurável para assinatura de links.Pra acompanhar a sugestão no MailTracker, proponho nova chave de config:
'fallback-event-listeners' => [ \jdavidbakr\MailTracker\Listener\DomainExistsInContentListener::class, ] + , + /** + * Tempo de vida (em segundos) da URL assinada de redirecionamento. Null => não expira. + */ + 'link-signature-ttl' => null,src/Middleware/ValidateSignature.php (1)
15-23: Validação de URL: restringir esquemas permitidos.Hoje qualquer esquema válido no RFC passa (ex.: mailto:, data:). Sugiro limitar a http/https (ou tornar configurável) para reduzir superfície de open‑redirect com esquemas esquisitos mesmo quando assinados.
Aplique algo assim:
- if (filter_var($url, FILTER_VALIDATE_URL) === false) { + if (filter_var($url, FILTER_VALIDATE_URL) === false) { throw new BadUrlLink('Mail hash: ' . $hash . ', URL: ' . $url); - } + } + $parts = parse_url($url); + if (! $parts || ! in_array(strtolower($parts['scheme'] ?? ''), ['http','https'], true)) { + throw new BadUrlLink('Mail hash: ' . $hash . ', URL: ' . $url); + }tests/MailTrackerTest.php (1)
46-52: Aviso do PHPMD sobre parâmetro não usado.Os métodos da handler de teste precisam do Throwable $e pela assinatura. Se o linter incomodar, podemos silenciar localmente com anotação.
Sugestão mínima:
- public function report(Throwable $e) + /** @SuppressWarnings(PHPMD.UnusedFormalParameter) */ + public function report(Throwable $e) { } - public function render($request, Throwable $e) + /** @SuppressWarnings(PHPMD.UnusedFormalParameter) */ + public function render($request, Throwable $e) { throw $e; }Also applies to: 50-53
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
README.md(6 hunks)config/mail-tracker.php(3 hunks)src/Events/ValidActionEvent.php(1 hunks)src/Events/ValidLinkEvent.php(1 hunks)src/Listener/DomainExistsInContentListener.php(1 hunks)src/MailTracker.php(2 hunks)src/MailTrackerController.php(3 hunks)src/MailTrackerServiceProvider.php(6 hunks)src/Middleware/ValidateSignature.php(1 hunks)src/Model/SentEmail.php(1 hunks)tests/MailTrackerControllerTest.php(3 hunks)tests/MailTrackerTest.php(38 hunks)tests/SetUpTest.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
src/Events/ValidLinkEvent.php (1)
src/MailTracker.php (1)
MailTracker(24-356)
src/Middleware/ValidateSignature.php (5)
src/MailTracker.php (2)
MailTracker(24-356)sentEmailModel(76-79)src/Events/ValidLinkEvent.php (1)
ValidLinkEvent(9-22)src/Exceptions/BadUrlLink.php (1)
BadUrlLink(7-10)src/Model/SentEmail.php (1)
SentEmail(26-70)src/Listener/DomainExistsInContentListener.php (1)
handle(9-18)
src/Listener/DomainExistsInContentListener.php (2)
src/Events/ValidLinkEvent.php (1)
ValidLinkEvent(9-22)src/Middleware/ValidateSignature.php (1)
handle(15-45)
src/MailTrackerServiceProvider.php (4)
src/MailTracker.php (1)
MailTracker(24-356)src/Events/ValidLinkEvent.php (1)
ValidLinkEvent(9-22)src/Listener/DomainExistsInContentListener.php (1)
DomainExistsInContentListener(7-19)src/Middleware/ValidateSignature.php (1)
ValidateSignature(13-46)
src/MailTrackerController.php (3)
src/MailTracker.php (2)
MailTracker(24-356)sentEmailModel(76-79)src/Events/ValidActionEvent.php (1)
ValidActionEvent(9-20)src/RecordLinkClickJob.php (1)
RecordLinkClickJob(13-64)
config/mail-tracker.php (1)
src/Listener/DomainExistsInContentListener.php (1)
DomainExistsInContentListener(7-19)
tests/MailTrackerTest.php (4)
src/MailTracker.php (3)
MailTracker(24-356)sentEmailModel(76-79)messageSent(117-128)src/Events/EmailSentEvent.php (1)
EmailSentEvent(10-26)src/RecordTrackingJob.php (1)
RecordTrackingJob(13-47)src/Exceptions/BadUrlLink.php (1)
BadUrlLink(7-10)
🪛 PHPMD (2.15.0)
tests/MailTrackerTest.php
46-46: Avoid unused parameters such as '$e'. (undefined)
(UnusedFormalParameter)
173-173: Avoid unused local variables such as '$name'. (undefined)
(UnusedLocalVariable)
195-195: Avoid unused local variables such as '$name'. (undefined)
(UnusedLocalVariable)
🪛 markdownlint-cli2 (0.18.1)
README.md
193-193: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
195-195: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
196-196: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
198-198: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
199-199: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
200-200: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
205-205: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
206-206: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
208-208: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
209-209: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
212-212: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
213-213: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (21)
tests/SetUpTest.php (1)
62-64: Mail em testes via log: boa escolha, meu truta.Definir
mail.default = 'log'no ambiente de teste evita side‑effects e torna os testes determinísticos. Sem observações.src/MailTracker.php (1)
12-12: Import do URL facade está correto.Necessário para
URL::signedRoute(...). Tudo certo aqui.src/Model/SentEmail.php (1)
20-24: Atualização de PHPDoc: show.Tipos nulos para meta e timestamps melhoram DX e ferramentas. Sem ressalvas.
README.md (1)
76-80: Exemplo no AppServiceProvider usando register(): ok.Colocar
MailTracker::ignoreMigrations()emregister()é apropriado. Tudo certo.config/mail-tracker.php (1)
108-113: Aprovado — fallback listeners (ValidLinkEvent) corretamente registrados e disparados.ValidateSignature dispara ValidLinkEvent quando a assinatura falha; MailTrackerServiceProvider registra os listeners via Event::listen; DomainExistsInContentListener existe e trata o evento. Ver: src/MailTrackerServiceProvider.php, src/Middleware/ValidateSignature.php, src/Listener/DomainExistsInContentListener.php — fala, meu truta, sinal verde.
tests/MailTrackerControllerTest.php (2)
7-7: Import do URL facade — tudo certo.Alinha os testes com o uso de URLs assinadas. Boa!
119-122: Uso da chave 'l' — perfeito.Bate com o middleware ValidateSignature e com a injeção feita no MailTracker.
src/Middleware/ValidateSignature.php (2)
31-45: Fallback por evento quando assinatura falha — desenho sólido.Resolve via ValidLinkEvent + listeners e redireciona para fallback quando inválido. Fluxo está consistente.
24-29: Checagem da assinatura — ok; confirmar existência de Request::hasValidSignatureWhileIgnoringFala meu truta — parseArguments + chamada parecem corretas. composer.json não referencia laravel/framework e não há vendor/ nem composer.lock; a única ocorrência da chamada é src/Middleware/ValidateSignature.php:27. Confirma localmente se Request::hasValidSignatureWhileIgnoring existe na instalação (vendor/) ou no Request usado; se não existir, troque por hasValidSignature (Laravel) ou adicione uma implementação compatível.
src/MailTrackerController.php (3)
16-20: Content-Length dinâmico no pixel — show.Evita número mágico e garante header correto.
30-31: Dispatch via facade Event — alinhado.Troca por Event::dispatch centraliza o padrão no pacote.
47-76: Fluxo do getN está redondo.
- Usa 'l'/'h' do request, dispara job de click, ajusta opened_at/clicked_at e salva uma vez.
- Redireciona sempre (tracking pode ser pulado sem quebrar UX).
Tudo coerente com o middleware.src/MailTrackerServiceProvider.php (3)
45-51: Listeners de fallback configuráveis — mandou bem.Registro do ValidLinkEvent com lista de listeners via config facilita extensão sem fork.
110-114: Rota 'n' com middleware ValidateSignature — certinho.Garante validação antes do controller e mantém o nome da rota usado pelo MailTracker/URL::signedRoute.
85-89: Confirmar typo: "emailTrakingViews" (sem 'c')Fala, meu truta — referências encontradas em: README.md, src/MailTrackerServiceProvider.php, src/views/emails/mensaje.blade.php, src/AdminController.php, config/mail-tracker.php. Mudar o namespace quebrará compatibilidade; se for legado, mantenha. Se for corrigir antes do release, atualize todas as referências e registre ambos os namespaces no ServiceProvider (emailTrakingViews + emailTrackingViews) ou documente a migração para evitar BC break.
tests/MailTrackerTest.php (6)
387-396: URL assinada no teste de link — ok.Cobre clique, fila e timestamps; cenário feliz está bem testado.
425-433: Caminho de fallback com rota não assinada — ótimo.Valida o redirect configurado quando hash inválido/sem assinatura.
438-450: Validação por domínio no conteúdo — cobertura boa.Exercita o DomainExistsInContentListener corretamente.
473-483: Entrada inválida em 'l' gera BadUrlLink — teste necessário.Traz segurança contra lixo/strings aleatórias.
846-871: Teste de ampersands — robusto.Regex + redirecionamento e contagem de cliques verificados. Tudo nos conformes.
928-936: Teste de apóstrofo nas URLs — beleza.Garante que a assinatura/redirect não se perde com caracteres especiais.
| ```php | ||
| 'log-content-strategy' => 'filesystem', | ||
| 'tracker-filesystem' => null | ||
| 'tracker-filesystem-folder' => 'mail-tracker', | ||
| ```` | ||
| To use the filesystem you need to change the `log-content-strategy` from `database` to `filesystem`. | ||
| ``` |
There was a problem hiding this comment.
Vírgula faltando no snippet de config (quebra PHP ao copiar/colar).
Entre as linhas 'tracker-filesystem' e 'tracker-filesystem-folder' falta uma vírgula.
- 'tracker-filesystem' => null
+ 'tracker-filesystem' => null,
'tracker-filesystem-folder' => 'mail-tracker',🤖 Prompt for AI Agents
In README.md around lines 110 to 114, the PHP config snippet is missing a comma
after the 'tracker-filesystem' line which will break PHP when copy/pasting; add
a trailing comma immediately after the 'tracker-filesystem' entry so the array
elements are properly separated and the snippet is valid PHP.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@composer.json`:
- Around line 28-29: The composer constraint for "orchestra/testbench" includes
an unstable release (^11.0); update the dependency in composer.json to remove
the ^11.0 alternative and only allow stable releases (e.g., change
"orchestra/testbench": "^8.0|^9.0|^10.0" or use an explicit stable range like
">=8.0 <11.0"), then run composer update to verify compatibility and confirm the
chosen version matches the Laravel target.
- Around line 20-30: Update the PHP requirement in composer.json from "php":
"^8.1" to "php": "^8.3" so the package guarantees compatibility with Laravel 13;
locate the "require" block in composer.json (the "php" key) and change its
version constraint to ^8.3, then run composer validate and composer update (or
update your CI matrix) to ensure dependencies resolve against PHP 8.3.
- Line 22: The composer.json currently pins unavailable future releases
("illuminate/support": "^13.0" and "orchestra/testbench": "^11.0") which will
break installs; update composer.json to remove the unsupported ^13.0 from the
"illuminate/support" constraint and replace or restrict "orchestra/testbench" to
the latest available stable (e.g., "10.9.0" or "^10.9") or a dev branch if
intentional, or alternatively add a clear inline comment next to the
"illuminate/support" and "orchestra/testbench" entries noting these entries are
reserved for future support and must be reverted/updated when those versions are
officially released (refer to the package keys "illuminate/support" and
"orchestra/testbench" to locate and change the constraints).
| "require": { | ||
| "php": "^8.1", | ||
| "illuminate/support": "^10.0|^11.0|^12.0", | ||
| "illuminate/support": "^10.0|^11.0|^12.0|^13.0", | ||
| "guzzlehttp/guzzle": "^7.2", | ||
| "aws/aws-php-sns-message-validator": "^1.8", | ||
| "aws/aws-sdk-php": "^3.258" | ||
| }, | ||
| "require-dev": { | ||
| "phpunit/phpunit": "^9.5.10|^10.5|^11.5.3", | ||
| "orchestra/testbench": "^8.0|^9.0|^10.0", | ||
| "phpunit/phpunit": "^9.5.10|^10.5|^11.5.3|^12.5.12", | ||
| "orchestra/testbench": "^8.0|^9.0|^10.0|^11.0", | ||
| "mockery/mockery": "^1.4.4" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Laravel 13 minimum PHP version requirement official documentation
💡 Result:
Laravel 13 requires PHP 8.3 or higher (supported PHP range listed as 8.3–8.5 in Laravel’s official Support Policy table). [1]
Ajustar requisito de PHP para Laravel 13, meu truta!
Meu brother, a preocupação é válida mesmo! Laravel 13 requer PHP 8.3 ou superior, mas o php: ^8.1 permite versões 8.1 e 8.2 que não são oficialmente suportadas. Para garantir compatibilidade total com o Laravel 13, muda pra php: ^8.3 no composer.json.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@composer.json` around lines 20 - 30, Update the PHP requirement in
composer.json from "php": "^8.1" to "php": "^8.3" so the package guarantees
compatibility with Laravel 13; locate the "require" block in composer.json (the
"php" key) and change its version constraint to ^8.3, then run composer validate
and composer update (or update your CI matrix) to ensure dependencies resolve
against PHP 8.3.
| "require": { | ||
| "php": "^8.1", | ||
| "illuminate/support": "^10.0|^11.0|^12.0", | ||
| "illuminate/support": "^10.0|^11.0|^12.0|^13.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Laravel 13 release date 2025 2026
💡 Result:
Laravel 13 is not a 2025 release. Laravel’s official release/support policy lists Laravel 13’s release as “Q1 2026” (no specific day announced there). Laravel 12 was released February 24, 2025, which is the current major release preceding 13. [1]
Answer: Laravel 13 release window: Q1 2026 (expected), not 2025. [1]
Sources:
[1] Laravel official docs – Release Notes / Support Policy (shows Laravel 13: “Q1 2026”)
🌐 Web query:
PHPUnit 12.5.12 packagist availability
💡 Result:
Yes—phpunit/phpunit 12.5.12 is available on Packagist (it appears in the published versions list for the package). You can install it via Composer with:
composer require --dev phpunit/phpunit:12.5.12[1]
🌐 Web query:
Orchestra Testbench 11.0 release
💡 Result:
As of February 27, 2026, Orchestra Testbench 11.0 has not been released (no v11.0.0 tag on Packagist). Packagist shows only an 11.x-dev development branch, while the latest stable release is v10.9.0 (released 2026-01-14). [1]
Source: Packagist package page for orchestra/testbench. [1]
Meu truta, duas dependências não foram lançadas ainda!
Fala! Encontrei um problema aqui no composer.json: o ^13.0 do illuminate/support e o ^11.0 do orchestra/testbench não estão disponíveis no Packagist ainda.
- Laravel 13: Esperado para Q1 2026 (não foi lançado ainda)
- Orchestra Testbench 11.0: Não existe (só tem a branch
11.x-devdisponível; a versão estável mais recente é a10.9.0lançada em 14 de janeiro de 2026)
Essas constraints vão quebrar a instalação do package até que essas versões sejam efetivamente lançadas. Recomendo remover essas versões futuras ou adicionar um comentário indicando que são para suporte futuro.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@composer.json` at line 22, The composer.json currently pins unavailable
future releases ("illuminate/support": "^13.0" and "orchestra/testbench":
"^11.0") which will break installs; update composer.json to remove the
unsupported ^13.0 from the "illuminate/support" constraint and replace or
restrict "orchestra/testbench" to the latest available stable (e.g., "10.9.0" or
"^10.9") or a dev branch if intentional, or alternatively add a clear inline
comment next to the "illuminate/support" and "orchestra/testbench" entries
noting these entries are reserved for future support and must be
reverted/updated when those versions are officially released (refer to the
package keys "illuminate/support" and "orchestra/testbench" to locate and change
the constraints).
| "phpunit/phpunit": "^9.5.10|^10.5|^11.5.3|^12.5.12", | ||
| "orchestra/testbench": "^8.0|^9.0|^10.0|^11.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verificar as versões disponíveis dos pacotes no Packagist
echo "=== PHPUnit 12.x versions ==="
curl -s https://packagist.org/packages/phpunit/phpunit.json | jq '.package.versions | keys[] | select(startswith("12."))' | sort -V
echo ""
echo "=== Orchestra Testbench 11.x versions ==="
curl -s https://packagist.org/packages/orchestra/testbench.json | jq '.package.versions | keys[] | select(startswith("11."))' | sort -VRepository: Accordous/mail-tracker
Length of output: 759
Ó meu truta, achei um problema sério na constraint do Orchestra Testbench!
E aí, parceiro! Verifiquei lá no Packagist e encontrei:
- ✅ PHPUnit 12.5.12: Existe sim, e usar
^12.5.12é válido (permite patches como 12.5.13, 12.5.14+) - ❌ Orchestra Testbench 11.0: Problema aqui, meu irmão! Não existem releases estáveis da v11 ainda — apenas
11.x-dev. A constraint^11.0tentará instalar a versão de desenvolvimento, o que pode causar instabilidade.
Recomendo verificar qual é a versão estável mais recente compatível com Laravel 13 ou aguardar um release estável do Testbench v11.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@composer.json` around lines 28 - 29, The composer constraint for
"orchestra/testbench" includes an unstable release (^11.0); update the
dependency in composer.json to remove the ^11.0 alternative and only allow
stable releases (e.g., change "orchestra/testbench": "^8.0|^9.0|^10.0" or use an
explicit stable range like ">=8.0 <11.0"), then run composer update to verify
compatibility and confirm the chosen version matches the Laravel target.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
62-68: ⚡ Quick winConsidera reforçar a segurança do workflow, meu brother!
Algumas práticas de segurança recomendadas que podem fortalecer o workflow:
Desabilitar persistência de credenciais: Adicionar
persist-credentials: falseno step de checkout previne vazamento de credenciais através de artifacts.Fixar ações por commit hash: Usar commit hashes específicos ao invés de tags (como
@v6,@v2) garante reprodutibilidade e protege contra ataques à supply chain.🛡️ Melhorias de segurança sugeridas
- name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v6 + with: + persist-credentials: false - name: Setup PHP - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@c541c155eee45413f5b09a52d31318011562d79b # v2 with: php-version: ${{ matrix.php }}Nota: Os hashes de commit acima são exemplos. Verifique os hashes reais das versões que deseja usar nos respectivos repositórios.
As per static analysis hints from zizmor and actionlint, que identificaram esses pontos de atenção.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/test.yml around lines 62 - 68, Atualize o workflow para fortalecer segurança: in the Checkout step (the step that uses actions/checkout@v6) add persist-credentials: false to prevent credential persistence, and replace the tag references actions/checkout@v6 and shivammathur/setup-php@v2 with explicit commit SHA pins for those actions (use the exact commit hash from the respective repos) to avoid supply-chain risks and ensure reproducibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test.yml:
- Around line 26-34: Atualize o comentário que atualmente diz "PHP 8.4 does not
support Laravel 10" para referir corretamente "PHP 8.5 does not support Laravel
10" e remova a entrada duplicada que exclui a combinação php: '8.5' + laravel:
'10.*' para que apenas uma exclusion block contendo php: '8.5' with laravel:
'10.*' remains; procure pelas linhas contendo the literal pairs "php: '8.5'" and
"laravel: '10.*'" para localizar e deduplicar a remoção.
- Line 67: Corrija a referência à variável da matriz usada no workflow:
substituir `${{ matrix.php-version }}` por `${{ matrix.php }}` para alinhar com
a propriedade definida na matriz (trocar a ocorrência de matrix.php-version para
matrix.php no bloco que define php-version).
---
Nitpick comments:
In @.github/workflows/test.yml:
- Around line 62-68: Atualize o workflow para fortalecer segurança: in the
Checkout step (the step that uses actions/checkout@v6) add persist-credentials:
false to prevent credential persistence, and replace the tag references
actions/checkout@v6 and shivammathur/setup-php@v2 with explicit commit SHA pins
for those actions (use the exact commit hash from the respective repos) to avoid
supply-chain risks and ensure reproducibility.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5fda5e08-19cb-4678-8cf8-8d0a5cf79db2
📒 Files selected for processing (12)
.github/workflows/test.ymlcomposer.jsontests/DeleteFileTest.phptests/MailTrackerControllerTest.phptests/MailTrackerTest.phptests/MigrateRecipientsTest.phptests/RecordBounceJobTest.phptests/RecordComplaintJobTest.phptests/RecordDeliveryJobTest.phptests/RecordLinkClickJobTest.phptests/RecordTrackingJobTest.phptests/TestCase.php
| # PHP 8.4 does not support Laravel 10 | ||
| - php: '8.5' | ||
| laravel: '10.*' | ||
|
|
||
| # PHP 8.5 does not support Laravel 11 or Laravel 10 | ||
| - php: '8.5' | ||
| laravel: '11.*' | ||
| - php: '8.5' | ||
| laravel: '10.*' |
There was a problem hiding this comment.
Corrige o comentário e remove a exclusão duplicada, meu caro amigo!
O comentário na linha 26 menciona PHP 8.4, mas a exclusão é para PHP 8.5 com Laravel 10. Além disso, as linhas 27-28 e 33-34 excluem a mesma combinação (PHP 8.5 + Laravel 10) — uma delas é redundante.
🔧 Correção sugerida
- # PHP 8.4 does not support Laravel 10
+ # PHP 8.5 does not support Laravel 10
- php: '8.5'
laravel: '10.*'
- # PHP 8.5 does not support Laravel 11 or Laravel 10
+ # PHP 8.5 does not support Laravel 11
- php: '8.5'
laravel: '11.*'
- - php: '8.5'
- laravel: '10.*'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # PHP 8.4 does not support Laravel 10 | |
| - php: '8.5' | |
| laravel: '10.*' | |
| # PHP 8.5 does not support Laravel 11 or Laravel 10 | |
| - php: '8.5' | |
| laravel: '11.*' | |
| - php: '8.5' | |
| laravel: '10.*' | |
| # PHP 8.5 does not support Laravel 10 | |
| - php: '8.5' | |
| laravel: '10.*' | |
| # PHP 8.5 does not support Laravel 11 | |
| - php: '8.5' | |
| laravel: '11.*' |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/test.yml around lines 26 - 34, Atualize o comentário que
atualmente diz "PHP 8.4 does not support Laravel 10" para referir corretamente
"PHP 8.5 does not support Laravel 10" e remova a entrada duplicada que exclui a
combinação php: '8.5' + laravel: '10.*' para que apenas uma exclusion block
contendo php: '8.5' with laravel: '10.*' remains; procure pelas linhas contendo
the literal pairs "php: '8.5'" and "laravel: '10.*'" para localizar e deduplicar
a remoção.
| - name: Setup PHP | ||
| uses: shivammathur/setup-php@v2 | ||
| with: | ||
| php-version: ${{ matrix.php-version }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verificar se a matriz define 'php' ou 'php-version'
grep -A 1 "matrix:" .github/workflows/test.yml | grep -E "^\s+php:"Repository: Accordous/mail-tracker
Length of output: 118
🏁 Script executed:
#!/bin/bash
# Visualizar a linha 67 e seu contexto
sed -n '60,75p' .github/workflows/test.yml | cat -nRepository: Accordous/mail-tracker
Length of output: 860
🏁 Script executed:
#!/bin/bash
# Contar linhas até encontrar a linha 67 exata
wc -l .github/workflows/test.yml
# Visualizar linhas 60-75 com números de linha exatos
sed -n '60,75p' .github/workflows/test.yml | nl -v 60Repository: Accordous/mail-tracker
Length of output: 157
🏁 Script executed:
#!/bin/bash
# Visualizar linhas 60-74 com cat -n
cat -n .github/workflows/test.yml | sed -n '60,74p'Repository: Accordous/mail-tracker
Length of output: 860
Corrige o nome da variável da matriz, meu truta!
A linha 67 usa ${{ matrix.php-version }}, mas a matriz define a propriedade como php (não php-version). Isso vai causar falha no workflow porque a variável não existe.
🐛 Correção proposta
with:
- php-version: ${{ matrix.php-version }}
+ php-version: ${{ matrix.php }}
coverage: none📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| php-version: ${{ matrix.php-version }} | |
| with: | |
| php-version: ${{ matrix.php }} | |
| coverage: none |
🧰 Tools
🪛 actionlint (1.7.12)
[error] 67-67: property "php-version" is not defined in object type {laravel: string; operating-system: string; php: number; phpunit: string}
(expression)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/test.yml at line 67, Corrija a referência à variável da
matriz usada no workflow: substituir `${{ matrix.php-version }}` por `${{
matrix.php }}` para alinhar com a propriedade definida na matriz (trocar a
ocorrência de matrix.php-version para matrix.php no bloco que define
php-version).
Summary by CodeRabbit
Notas de Lançamento
New Features
Bug Fixes
Chores