diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 00000000..836ae23b --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,73 @@ +name: CI + +on: + push: + branches: [main, backend-audit] + pull_request: + +jobs: + backend: + name: PHP — phpstan + migrate-on-empty-db + phpunit + runs-on: ubuntu-latest + + services: + mariadb: + image: mariadb:11.8 + env: + MARIADB_ROOT_PASSWORD: root + MARIADB_DATABASE: db + MARIADB_USER: db + MARIADB_PASSWORD: db + ports: + - 3306:3306 + options: >- + --health-cmd="healthcheck.sh --connect --innodb_initialized" + --health-interval=10s --health-timeout=5s --health-retries=20 + redis: + image: redis:7 + ports: + - 6379:6379 + options: --health-cmd="redis-cli ping" --health-interval=10s --health-timeout=5s --health-retries=10 + + env: + # Real env vars take precedence over .env/.env.test. doctrine's when@test + # config appends `_test`, so this `db` becomes `db_test` for the test run. + DATABASE_URL: "mysql://db:db@127.0.0.1:3306/db?serverVersion=11.8.0-MariaDB&charset=utf8mb4" + REDIS_URL: "redis://127.0.0.1:6379" + APP_ENV: test + + steps: + - uses: actions/checkout@v4 + + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: '8.3' + extensions: pdo_mysql, intl, redis, gd, zip, mbstring, bcmath + coverage: none + + - name: Install dependencies + run: composer install --no-interaction --prefer-dist --no-progress + + - name: Create test database + run: | + mysql -h127.0.0.1 -uroot -proot -e "CREATE DATABASE IF NOT EXISTS db_test; GRANT ALL ON db_test.* TO 'db'@'%'; FLUSH PRIVILEGES;" + + - name: Generate JWT keypair + run: php bin/console lexik:jwt:generate-keypair --skip-if-exists --env=test + + # phpstan's symfony extension reads the dev container XML (see phpstan.neon), + # so warm the dev cache first. + - name: Warm dev cache (for phpstan container) + run: php bin/console cache:warmup --env=dev + env: + APP_ENV: dev + + - name: PHPStan (baselined — fails only on NEW errors) + run: php vendor/bin/phpstan analyse --no-progress + + - name: Migrate on empty DB (smoke) + run: php bin/console doctrine:migrations:migrate --no-interaction --env=test + + - name: PHPUnit + run: php bin/phpunit diff --git a/docs/audit-backlog.md b/docs/audit-backlog.md index da704935..61b1a645 100644 --- a/docs/audit-backlog.md +++ b/docs/audit-backlog.md @@ -106,8 +106,8 @@ _None outstanding._ | E1 | **Double-nest response contract** — ~12 controllers return `success(['data'=>X])` → `{data:{data:X}}`, others flat. Inconsistent envelope. | AppointmentSettings, Appointment, Rating, Settlement, Secretary, Sms, Location, Doctor, Blog, DoctorService/Specialty/Tag, Representation, UserProfile, Clinic, Insurance, Billing | Documented pitfall in CLAUDE.md; clients (`nobat724_front`, `clinic-pro-tauri`) already read `data.data`. Unifying breaks all 3 clients → needs coordinated cross-repo change + versioning. NOT a quick bug. | | E2 | **No DTO/validator on sensitive writes** — admin create, auth flows, payment verify, booking read raw `json_decode` arrays | Admin, Auth, Payment, Appointment, Settlement controllers | Introduce request DTOs + validator incrementally. Large. | | E3 | **Fat controllers** — AdminApiController (1938 LOC), MyAppointmentsController booking, RepresentationActionController (835), DoctorController/ClinicController | extract per-domain Services | SOLID refactor; lower urgency than security/db. | -| E4 | **CI** — no `.github/workflows`; add phpunit + phpstan + migrate-on-empty-db | devops | prompt var §DevOps | -| E5 | **phpstan baseline dirty** — 41 pre-existing errors across the codebase (D9 only repaired the config so it *runs*). Audit fixes must not add new ones; cleaning the 41 is its own task. | devops | `ddev exec php vendor/bin/phpstan analyse` → 41 errors (e.g. SlotCalculatorService.php:233, SubscriptionController.php:33) | +| ✅E4 | **CI** — no `.github/workflows`; add phpunit + phpstan + migrate-on-empty-db | devops | **DONE (scaffold, first-run-pending)** — `.github/workflows/ci.yml`: MariaDB+redis services, phpstan (baseline-clean), migrate-on-empty-db smoke, phpunit. Verified locally: empty-DB migrate → 64 tables, phpstan OK, 70 tests green. GH-Actions service wiring needs first-run validation. | +| ✅E5 | **phpstan baseline dirty** — 41 pre-existing errors across the codebase (D9 only repaired the config so it *runs*). Audit fixes must not add new ones; cleaning the 41 is its own task. | devops | **DONE** — generated `phpstan-baseline.neon` (41 errors) + included it; `phpstan analyse` now returns OK, so the gate catches only NEW errors. Burn down over time. | | E6 | **No test DB isolation** — `ApiTestCase` doesn't reset/rollback `db_test` between tests/runs, so rows accumulate; count/time-based assertions are fragile (hit twice this session). Add per-test transaction rollback or a DB reset. | test | tests rely on random keys + relaxed assertions as a workaround | --- diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon new file mode 100644 index 00000000..32cf6bfa --- /dev/null +++ b/phpstan-baseline.neon @@ -0,0 +1,157 @@ +parameters: + ignoreErrors: + - + message: '#^Strict comparison using \!\=\= between mixed and null will always evaluate to true\.$#' + identifier: notIdentical.alwaysTrue + count: 1 + path: src/Appointment/Service/SlotCalculatorService.php + + - + message: '#^Offset ''role'' on array\{type\: ''clinic'', db_uuid\: mixed, name\: non\-falsy\-string, role\: ''secretary'', scope\: ''clinic'', permissions\: mixed\}\|array\{type\: ''clinic'', db_uuid\: string, name\: string, role\: ''clinic''\|''doctor'', scope\: ''clinic''\|null, doctor_uuid\: string\}\|array\{type\: ''clinic'', db_uuid\: string, name\: string, role\: ''clinic''\}\|array\{type\: ''doctor'', db_uuid\: mixed, name\: non\-falsy\-string, role\: ''secretary'', scope\: ''doctor'', permissions\: mixed\}\|array\{type\: ''doctor'', db_uuid\: string, name\: non\-falsy\-string, role\: ''doctor''\} on left side of \?\? always exists and is not nullable\.$#' + identifier: nullCoalesce.offset + count: 1 + path: src/Auth/Controller/AuthController.php + + - + message: '#^Call to an undefined method Symfony\\Component\\Security\\Core\\User\\UserInterface\:\:getId\(\)\.$#' + identifier: method.notFound + count: 2 + path: src/Auth/Security/PasswordAuthenticator.php + + - + message: '#^Call to an undefined method Symfony\\Component\\Security\\Core\\User\\UserInterface\:\:isStaff\(\)\.$#' + identifier: method.notFound + count: 1 + path: src/Auth/Security/PasswordAuthenticator.php + + - + message: '#^Call to an undefined method Symfony\\Contracts\\Cache\\CacheInterface\:\:getItem\(\)\.$#' + identifier: method.notFound + count: 1 + path: src/Auth/Security/PasswordAuthenticator.php + + - + message: '#^Call to an undefined method Symfony\\Contracts\\Cache\\CacheInterface\:\:save\(\)\.$#' + identifier: method.notFound + count: 1 + path: src/Auth/Security/PasswordAuthenticator.php + + - + message: '#^Call to an undefined method Symfony\\Contracts\\Cache\\CacheInterface\:\:getItem\(\)\.$#' + identifier: method.notFound + count: 4 + path: src/Auth/Service/OtpService.php + + - + message: '#^Call to an undefined method Symfony\\Contracts\\Cache\\CacheInterface\:\:save\(\)\.$#' + identifier: method.notFound + count: 3 + path: src/Auth/Service/OtpService.php + + - + message: '#^Call to an undefined method Symfony\\Contracts\\Cache\\CacheInterface\:\:getItem\(\)\.$#' + identifier: method.notFound + count: 2 + path: src/Auth/Service/TokenService.php + + - + message: '#^Call to an undefined method Symfony\\Contracts\\Cache\\CacheInterface\:\:save\(\)\.$#' + identifier: method.notFound + count: 1 + path: src/Auth/Service/TokenService.php + + - + message: '#^Match expression does not handle remaining value\: string$#' + identifier: match.unhandled + count: 1 + path: src/Billing/Controller/BillingController.php + + - + message: '#^Parameter \#3 \$filters of method App\\Billing\\Repository\\ClaimRepository\:\:findByTenant\(\) expects array\{status\?\: string\|null, insurance_id\?\: int\|null, from\?\: int\|null, to\?\: int\|null, q\?\: string\|null\}, array\{status\: non\-falsy\-string\|null, insurance_id\: non\-falsy\-string\|null, from\: non\-falsy\-string\|null, to\: non\-falsy\-string\|null, q\: non\-falsy\-string\|null\} given\.$#' + identifier: argument.type + count: 1 + path: src/Billing/Controller/BillingController.php + + - + message: '#^Using nullsafe property access "\?\-\>franchiseRials" on left side of \?\? is unnecessary\. Use \-\> instead\.$#' + identifier: nullsafe.neverNull + count: 2 + path: src/Billing/Service/BillingCalculator.php + + - + message: '#^Call to protected method getEntityManager\(\) of class Doctrine\\ORM\\EntityRepository\\.$#' + identifier: method.protected + count: 1 + path: src/Billing/Service/ClaimService.php + + - + message: '#^Strict comparison using \=\=\= between App\\Auth\\Entity\\User and null will always evaluate to false\.$#' + identifier: identical.alwaysFalse + count: 1 + path: src/Blog/Entity/Blog.php + + - + message: '#^Call to protected method getEntityManager\(\) of class Doctrine\\ORM\\EntityRepository\\.$#' + identifier: method.protected + count: 1 + path: src/Insurance/Controller/InsuranceController.php + + - + message: '#^Strict comparison using \!\=\= between mixed and null will always evaluate to true\.$#' + identifier: notIdentical.alwaysTrue + count: 4 + path: src/Insurance/Controller/InsuranceController.php + + - + message: '#^Property App\\Patient\\Service\\PatientService\:\:\$userRepo is never read, only written\.$#' + identifier: property.onlyWritten + count: 1 + path: src/Patient/Service/PatientService.php + + - + message: '#^Offset 0 on non\-empty\-list\ on left side of \?\? always exists and is not nullable\.$#' + identifier: nullCoalesce.offset + count: 1 + path: src/Payment/Gateway/MellatGateway.php + + - + message: '#^Call to an undefined method Symfony\\Contracts\\Cache\\CacheInterface\:\:deleteItem\(\)\.$#' + identifier: method.notFound + count: 2 + path: src/Payment/Service/CircuitBreakerService.php + + - + message: '#^Call to an undefined method Symfony\\Contracts\\Cache\\CacheInterface\:\:getItem\(\)\.$#' + identifier: method.notFound + count: 3 + path: src/Payment/Service/CircuitBreakerService.php + + - + message: '#^Call to an undefined method Symfony\\Contracts\\Cache\\CacheInterface\:\:save\(\)\.$#' + identifier: method.notFound + count: 2 + path: src/Payment/Service/CircuitBreakerService.php + + - + message: '#^Call to an undefined method Symfony\\Contracts\\Cache\\CacheInterface\:\:getItem\(\)\.$#' + identifier: method.notFound + count: 1 + path: src/Shared/Controller/HealthController.php + + - + message: '#^Call to protected method getEntityManager\(\) of class Doctrine\\ORM\\EntityRepository\\.$#' + identifier: method.protected + count: 1 + path: src/Sms/Command/SeedSmsMessageTemplatesCommand.php + + - + message: '#^Property App\\Payment\\Gateway\\PaymentInitResult\:\:\$errorMessage \(string\) on left side of \?\? is not nullable\.$#' + identifier: nullCoalesce.property + count: 1 + path: src/Sms/Controller/SmsWalletController.php + + - + message: '#^Property App\\Subscription\\Controller\\SubscriptionController\:\:\$subscriptionRepo is never read, only written\.$#' + identifier: property.onlyWritten + count: 1 + path: src/Subscription/Controller/SubscriptionController.php diff --git a/phpstan.neon b/phpstan.neon index c5bd4bb7..9e8050c1 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,6 +1,9 @@ includes: - vendor/phpstan/phpstan-symfony/extension.neon - vendor/phpstan/phpstan-doctrine/extension.neon + # Pre-existing errors are baselined (E5 in docs/audit-backlog.md) so the gate + # enforces "no NEW errors". Burn the baseline down over time. + - phpstan-baseline.neon parameters: level: 5