mirror of
https://github.com/karatektus/pl_mail.git
synced 2026-10-25 01:54:10 +00:00
Add PHPStan and Dependabot, and fix two bugs they found
55k lines of PHP with no static analysis, and a dependency tree holding four OAuth clients, a JWT library and an IMAP parser with nothing watching it for advisories. PHPStan at level 5 with a baseline. Level 5 is where the class of defect this codebase actually has starts showing up — nullable entity getters flowing into non-nullable parameters — without a fortnight of triage first. 208 findings are baselined as a debt ledger, not a licence; the level goes up when the ledger is small enough that raising it is a day's work. Two of the 217 were real bugs, both fixed here rather than baselined: "Test connection" in account settings constructed ConnectionTestResult with four arguments where six are required, and passed null for a string parameter. Both call sites are the early returns for an incomplete form and a blank password — that is, the two cases the button exists to report, each of which raised an ArgumentCountError instead. GraphCategorySyncer caught App\Service\Mail\GraphApiException, which does not exist; the client throws App\Domain\Exception\GraphApiException. The catch never matched, so the 403 that means "this account needs reconnecting with MailboxSettings.ReadWrite" propagated as an unhandled error instead of degrading quietly, which is what the comment inside it says it is there to do. One more it found is left alone deliberately, because it needs a decision rather than a fix: Controller/Admin/User/DefaultController references App\Form\Admin\UserFormType and User::setName(), neither of which exists, so that admin page fatals on load. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
262fc95270
commit
ddccc8fa2e
8 files changed
+1282
-1
No files matched your search
@@ -0,0 +1,51 @@
|
||||
version: 2
|
||||
|
||||
# A mail client's dependency tree is its attack surface: four OAuth clients, a
|
||||
# JWT library, an IMAP parser and an HTML sanitiser, all handling bytes that
|
||||
# arrive from outside. Nothing was watching them.
|
||||
#
|
||||
# Grouped and weekly rather than per-package and daily. plMail pins Symfony to
|
||||
# a single minor (8.1.*), so its packages move together and one PR per package
|
||||
# would be a dozen PRs that only make sense merged at once.
|
||||
updates:
|
||||
- package-ecosystem: composer
|
||||
directory: /
|
||||
schedule:
|
||||
interval: weekly
|
||||
open-pull-requests-limit: 5
|
||||
groups:
|
||||
symfony:
|
||||
patterns:
|
||||
- 'symfony/*'
|
||||
- 'symfonycasts/*'
|
||||
doctrine:
|
||||
patterns:
|
||||
- 'doctrine/*'
|
||||
# Everything else together: these are the ones worth reading
|
||||
# individually, and there are few enough of them to do that.
|
||||
minor-and-patch:
|
||||
update-types:
|
||||
- minor
|
||||
- patch
|
||||
|
||||
- package-ecosystem: npm
|
||||
directory: /
|
||||
schedule:
|
||||
interval: weekly
|
||||
open-pull-requests-limit: 3
|
||||
groups:
|
||||
playwright:
|
||||
patterns:
|
||||
- '@playwright/*'
|
||||
- 'playwright'
|
||||
|
||||
# The build publishes the image users actually run, so a stale or
|
||||
# vulnerable action here reaches further than a stale library.
|
||||
- package-ecosystem: github-actions
|
||||
directory: /
|
||||
schedule:
|
||||
interval: weekly
|
||||
groups:
|
||||
actions:
|
||||
patterns:
|
||||
- '*'
|
||||
@@ -142,6 +142,12 @@ jobs:
|
||||
php bin/console app:test:seed-user
|
||||
php bin/console app:test:seed-user --admin --email=e2e-admin@plmail.test --password=e2e-admin-password
|
||||
|
||||
# Needs the compiled test container for phpstan-symfony to resolve
|
||||
# service ids, and a booted kernel for the Doctrine metadata — both
|
||||
# of which the seeding above has already produced.
|
||||
- name: Run PHPStan
|
||||
run: composer stan
|
||||
|
||||
# Fast and dependency-free, so it runs before the browser suite:
|
||||
# a failure here is far quicker to read than a Playwright trace.
|
||||
- name: Run PHPUnit tests
|
||||
|
||||
File diff suppressed because it is too large.
Load diff
@@ -0,0 +1,38 @@
|
||||
# Static analysis for src/ and tests/.
|
||||
#
|
||||
# Level 5 to start, with everything already failing captured in a baseline. The
|
||||
# alternative — level 0 with no baseline — reports nothing interesting, and
|
||||
# level 9 buys a week of triage before it finds its first real bug. Five is
|
||||
# where the class of defect this codebase actually has starts showing up:
|
||||
# nullable entity getters (`?int getId()`, `?Account getAccount()`) flowing into
|
||||
# parameters and return types that are not nullable.
|
||||
#
|
||||
# The baseline is a debt ledger, not a licence. It shrinks; raise the level when
|
||||
# it is small enough that doing so is a day's work rather than a fortnight's.
|
||||
includes:
|
||||
- phpstan-baseline.neon
|
||||
- vendor/phpstan/phpstan-symfony/extension.neon
|
||||
- vendor/phpstan/phpstan-symfony/rules.neon
|
||||
- vendor/phpstan/phpstan-doctrine/extension.neon
|
||||
- vendor/phpstan/phpstan-doctrine/rules.neon
|
||||
|
||||
parameters:
|
||||
level: 5
|
||||
|
||||
paths:
|
||||
- src
|
||||
- tests
|
||||
|
||||
# Generated: 54 migrations nobody edits by hand, and a 1000-line dump of
|
||||
# the framework config tree that exists to be read, not analysed.
|
||||
excludePaths:
|
||||
analyseAndScan:
|
||||
- migrations
|
||||
- config/reference.php
|
||||
|
||||
symfony:
|
||||
containerXmlPath: var/cache/test/App_KernelTestDebugContainer.xml
|
||||
consoleApplicationLoader: tests/phpstan-console-loader.php
|
||||
|
||||
doctrine:
|
||||
objectManagerLoader: tests/phpstan-object-manager.php
|
||||
@@ -220,8 +220,10 @@ final class AccountController extends AbstractController
|
||||
return $this->json(new ConnectionTestResult(
|
||||
false,
|
||||
'Enter at least an email address and an IMAP host first.',
|
||||
'',
|
||||
null,
|
||||
'',
|
||||
'',
|
||||
)->toArray());
|
||||
}
|
||||
|
||||
@@ -229,8 +231,10 @@ final class AccountController extends AbstractController
|
||||
return $this->json(new ConnectionTestResult(
|
||||
false,
|
||||
'No password available to test with. Enter one above — on the edit form a blank field means "keep the stored password", which the tester can only resolve once the account id reaches it.',
|
||||
'',
|
||||
null,
|
||||
'',
|
||||
'',
|
||||
)->toArray());
|
||||
}
|
||||
|
||||
|
||||
@@ -6,8 +6,8 @@ namespace App\Service\Graph;
|
||||
|
||||
use App\Entity\Mail\Account;
|
||||
use App\Service\Label\LabelResolver;
|
||||
use App\Domain\Exception\GraphApiException;
|
||||
use App\Service\Mail\GraphApiClient;
|
||||
use App\Service\Mail\GraphApiException;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
use Psr\Log\LoggerInterface;
|
||||
|
||||
|
||||
@@ -0,0 +1,16 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
// Lets phpstan-symfony understand console commands — argument and option types
|
||||
// on InputInterface::getArgument()/getOption(), which are otherwise mixed.
|
||||
|
||||
use App\Kernel;
|
||||
use Symfony\Bundle\FrameworkBundle\Console\Application;
|
||||
use Symfony\Component\Dotenv\Dotenv;
|
||||
|
||||
require dirname(__DIR__) . '/vendor/autoload.php';
|
||||
|
||||
new Dotenv()->bootEnv(dirname(__DIR__) . '/.env');
|
||||
|
||||
return new Application(new Kernel('test', true));
|
||||
@@ -0,0 +1,19 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
// Lets phpstan-doctrine resolve entity metadata, so it can check DQL strings
|
||||
// and repository return types instead of treating them as mixed.
|
||||
|
||||
use App\Kernel;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
use Symfony\Component\Dotenv\Dotenv;
|
||||
|
||||
require dirname(__DIR__) . '/vendor/autoload.php';
|
||||
|
||||
new Dotenv()->bootEnv(dirname(__DIR__) . '/.env');
|
||||
|
||||
$kernel = new Kernel('test', true);
|
||||
$kernel->boot();
|
||||
|
||||
return $kernel->getContainer()->get('doctrine')->getManager();
|
||||
Reference in new issue
Block a user