From aba7748d3cde2b4e635a82a58cd6549c03b886c4 Mon Sep 17 00:00:00 2001 From: "Beau Beauchamp, WebTigers" Date: Fri, 24 Jul 2026 14:33:11 -0400 Subject: [PATCH 1/2] test(infra): coverage CI + savepoint isolation + Authorization gate (Wave-3 tail) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three things, +15 integration tests (265 green): 1. Coverage in CI — a `coverage` job (pcov, full suite) publishes a line-% summary to each PR's job-summary panel and enforces a ratcheting MIN_COVERAGE floor (report- above, fail-below), separate from the gating jobs so pcov never slows them. Baseline ~20% lines; target 90% — the floor bumps up as the backfill climbs, never down. 2. SavepointAdapter (resolves COVERAGE-PLAN §9 finding #7) — a test-only adapter that maps nested begin/commit/rollBack onto MySQL SAVEPOINTs, so a service's own _transaction() composes inside the per-test outer transaction instead of throwing "already an active transaction", and the outer rollback still discards everything. Wired into IntegrationTestCase. SavepointIsolationTest (6) locks it — incl. a real Access_Service_User::save nesting with no commit-and-purge. Unblocks clean service happy-path testing for every future wave. 3. Tiger_Controller_Plugin_Authorization (9) — the unbypassable deny-by-default gate's brains: _resolveRole() reads the role LIVE from org_user (session role ignored; revoked membership drops to base next request; locked session -> guest; actor/org stamped) and _resourceFor() controller->resource mapping. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/tests.yml | 66 +++++++ tests/COVERAGE-PLAN.md | 28 ++- .../Controller/AuthorizationPluginTest.php | 161 ++++++++++++++++++ tests/Integration/SavepointIsolationTest.php | 121 +++++++++++++ tests/Support/IntegrationTestCase.php | 4 +- tests/Support/SavepointAdapter.php | 76 +++++++++ 6 files changed, 451 insertions(+), 5 deletions(-) create mode 100644 tests/Integration/Controller/AuthorizationPluginTest.php create mode 100644 tests/Integration/SavepointIsolationTest.php create mode 100644 tests/Support/SavepointAdapter.php diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 83b2afd..c8052cb 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -82,3 +82,69 @@ jobs: - name: Integration suite (real MariaDB schema) run: composer test:integration + + # Full suite under pcov → a coverage summary on every PR (report-only; we ratchet MIN_COVERAGE up as + # the backfill climbs toward the 90% target). Separate from the gating jobs so pcov never slows them. + coverage: + runs-on: ubuntu-latest + timeout-minutes: 20 + services: + mariadb: + image: mariadb:10.11 + env: + MARIADB_ROOT_PASSWORD: root + MARIADB_DATABASE: tiger_test + ports: ['3306:3306'] + options: >- + --health-cmd="healthcheck.sh --connect --innodb_initialized" + --health-interval=10s + --health-timeout=5s + --health-retries=15 + env: + TIGER_TEST_DB_HOST: 127.0.0.1 + TIGER_TEST_DB_PORT: 3306 + TIGER_TEST_DB_NAME: tiger_test + TIGER_TEST_DB_USER: root + TIGER_TEST_DB_PASS: root + # The floor we enforce today (report-only warning above it). Bump this as coverage climbs so it + # can only go UP — the ratchet that gets us to 90% without ever regressing. + MIN_COVERAGE: '18' + steps: + - uses: actions/checkout@v4 + + - name: Set up PHP (pcov) + uses: shivammathur/setup-php@v2 + with: + php-version: '8.3' + extensions: mbstring, intl, pdo_mysql, curl, zip, gd, sodium, apcu + ini-values: apc.enable_cli=1, memory_limit=512M + tools: composer:v2 + coverage: pcov + + - name: Install dependencies + run: composer install --no-interaction --no-progress --prefer-dist + + - name: Full suite with coverage + run: | + vendor/bin/phpunit --coverage-text=coverage.txt --coverage-clover=clover.xml --colors=never + + - name: Publish coverage summary + if: always() + run: | + # The overall line-coverage % from Clover (covered / total elements). + PCT=$(php -r ' + $x = simplexml_load_file("clover.xml"); + $m = $x->project->metrics; + $c = (int)$m["coveredstatements"]; $t = (int)$m["statements"]; + printf("%.1f", $t ? 100*$c/$t : 0); + ') + echo "## Code coverage: ${PCT}% lines" >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + echo "Floor (MIN_COVERAGE): ${MIN_COVERAGE}% — target: 90%." >> "$GITHUB_STEP_SUMMARY" + echo '```' >> "$GITHUB_STEP_SUMMARY" + sed -n '/Summary:/,/^$/p' coverage.txt >> "$GITHUB_STEP_SUMMARY" + echo '```' >> "$GITHUB_STEP_SUMMARY" + echo "Line coverage: ${PCT}% (floor ${MIN_COVERAGE}%, target 90%)" + # Report-only fail guard against the floor, so coverage can never silently regress below it. + php -r 'exit((float)$argv[1] + 1e-9 < (float)$argv[2] ? 1 : 0);' "$PCT" "$MIN_COVERAGE" \ + || { echo "::error::Coverage ${PCT}% fell below the ${MIN_COVERAGE}% floor"; exit 1; } diff --git a/tests/COVERAGE-PLAN.md b/tests/COVERAGE-PLAN.md index a4d02d2..f88d915 100644 --- a/tests/COVERAGE-PLAN.md +++ b/tests/COVERAGE-PLAN.md @@ -561,10 +561,30 @@ Then **4 parallel agents** (own worktree + own DB `tiger_test_w3a-d`, off `test/ - **D / auth engine** — `Tiger_Service_Authentication` (password login, lockout, pepper, one-time challenges, 2FA orchestration). Collect → one DB → fold into ONE PR (dodges stacked-squash pain, per Waves 1+2). -### Next waves (unwritten — priority order per §5/§8) -- **Wave 3 tail:** `Tiger_Controller_Plugin_Authorization` (the unbypassable front-controller ACL gate). -- **Wave 4 — satellite repos:** stand up a harness in each, then TigerShield WAF engines (`Waf`/`Blocklist`/ - `RateLimit`/`Challenge` — highest-value non-core), TigerDocs, then the commerce module repos. +### Wave 3 tail + test-infra (LANDED 2026-07-24) → 265 integration green +- **Finding #7 RESOLVED — re-entrant transaction isolation.** `tests/Support/SavepointAdapter.php` (a + test-only `Zend_Db_Adapter_Pdo_Mysql` subclass) maps nested `begin/commit/rollBack` onto MySQL + SAVEPOINTs, so a service's own `_transaction()` (or a model `save()`) composes inside the per-test + outer transaction instead of throwing — and the outer rollback still discards everything. Wired into + `IntegrationTestCase::adapter()`. `SavepointIsolationTest` (6) locks it, incl. a real + `Access_Service_User::save` nesting with no commit-and-purge. **Future service tests no longer need the + escape-and-scrub workaround** — dispatch inside the base txn and rely on rollback isolation. +- **`Tiger_Controller_Plugin_Authorization`** (9) — the live-role guarantee: `_resolveRole()` reads the + role FRESH from `org_user` (session role is ignored; a revoked membership drops to base next request; a + locked session → guest; actor/org stamped) + `_resourceFor()` controller→resource mapping. The + preDispatch→redirect/403 cycle (front controller + exiting redirector) stays a functional/smoke concern. +- **Coverage in CI.** `.github/workflows/tests.yml` gained a `coverage` job (pcov, full suite) that + publishes a line-% summary to the PR and enforces a ratcheting `MIN_COVERAGE` floor. **Baseline ≈20% + lines** (2863/14397) — the tested spine is ~100%, the drag is untested feature modules (blog/seo/media/ + profile/analytics/backup/identity/schedule at 0%) + the marketplace/install cluster. **Target: 90%.** + +### Next waves (priority order per §5/§8) — the drive to 90% +- **Wave 4 — breadth over the zero-coverage core modules** (now unblocked by the savepoint mode): each is + small — media, profile, identity, blog, seo, analytics, backup, schedule, ally, search, agent — parallel + agents, one module (or a cluster) each, service + model + controller coverage. Biggest, fastest % gain. +- **Wave 5 — finish the kernel install/marketplace cluster** (`Module_Installer`, `Vendor`, `Update_Checker`, + `Module_Github`, `Module_Dependency`) — the largest untested library chunk, security-adjacent. +- **Wave 6 — satellite repos:** stand up a harness in each, then TigerShield WAF engines first. --- *Manifest generated 2026-07-18 by 6 parallel read-only scans; graduated into `tests/COVERAGE-PLAN.md` + diff --git a/tests/Integration/Controller/AuthorizationPluginTest.php b/tests/Integration/Controller/AuthorizationPluginTest.php new file mode 100644 index 0000000..65bbca0 --- /dev/null +++ b/tests/Integration/Controller/AuthorizationPluginTest.php @@ -0,0 +1,161 @@ +priorUnitTestMode = Zend_Session::$_unitTestEnabled; + Zend_Session::$_unitTestEnabled = true; + $_SESSION = []; + } + + protected function tearDown(): void + { + $_SESSION = []; + Zend_Session::$_unitTestEnabled = $this->priorUnitTestMode; + parent::tearDown(); + } + + /** A subclass that exposes the two protected decision methods for direct testing. */ + private function plugin(): Tiger_Controller_Plugin_Authorization + { + return new class extends Tiger_Controller_Plugin_Authorization { + public function pubResolveRole() { return $this->_resolveRole(); } + public function pubResourceFor($request) { return $this->_resourceFor($request); } + }; + } + + /** Seed a user + org + an active membership carrying $role; sign the user in. Returns [userId, orgId]. */ + private function signInWithMembership(string $role): array + { + $userId = (new Tiger_Model_User())->insert(['email' => $role . '@authz.test', 'status' => 'active']); + $orgId = (new Tiger_Model_Org())->insert(['name' => 'Authz ' . $role, 'slug' => 'authz-' . $role]); + (new Tiger_Model_OrgUser())->insert(['org_id' => $orgId, 'user_id' => $userId, 'role' => $role]); + // The identity deliberately claims a DIFFERENT role, to prove resolveRole reads the LIVE one. + $this->login($userId, $orgId, 'user'); + return [$userId, $orgId]; + } + + private function req(string $controller, string $module = '', string $action = 'index'): Zend_Controller_Request_Simple + { + $r = new Zend_Controller_Request_Simple(); + $r->setControllerName($controller)->setActionName($action); + if ($module !== '') { $r->setModuleName($module); } + return $r; + } + + // ----- _resolveRole (the live-role guarantee) ---------------------------------------------- + + #[Test] + public function a_request_with_no_identity_resolves_to_guest(): void + { + $this->logout(); + $this->assertSame('guest', $this->plugin()->pubResolveRole()); + } + + #[Test] + public function the_role_is_read_live_from_the_membership_not_the_session(): void + { + $this->signInWithMembership('manager'); // identity says 'user', membership says 'manager' + $this->assertSame('manager', $this->plugin()->pubResolveRole(), 'the LIVE org_user role wins over the session role'); + } + + #[Test] + public function revoking_the_membership_drops_to_the_base_authenticated_role_next_request(): void + { + [$userId, $orgId] = $this->signInWithMembership('admin'); + $this->assertSame('admin', $this->plugin()->pubResolveRole()); + + // Revoke the membership (soft-delete) — the very next resolve must fall back to the base role. + (new Tiger_Model_OrgUser())->softDelete(['org_id = ?' => $orgId, 'user_id = ?' => $userId]); + $this->assertSame('user', $this->plugin()->pubResolveRole(), 'a revoked membership takes effect immediately'); + } + + #[Test] + public function a_locked_session_resolves_to_guest(): void + { + $this->signInWithMembership('admin'); + // Suspend the session (lock screen) — authorize as guest everywhere until re-verified. + (new \Tiger_Service_Authentication())->lock(); + $this->assertSame('guest', $this->plugin()->pubResolveRole(), 'a locked session is treated as guest'); + } + + #[Test] + public function resolving_the_role_stamps_the_model_actor_and_org(): void + { + Tiger_Model_Table::setActor(null); + Tiger_Model_Table::setOrg(''); + [$userId, $orgId] = $this->signInWithMembership('manager'); + + $this->plugin()->pubResolveRole(); + + $this->assertSame($userId, Tiger_Model_Table::actor(), 'the acting user is stamped for created_by/updated_by'); + $this->assertSame($orgId, Tiger_Model_Table::org(), 'the active org (tenant) is stamped'); + } + + // ----- _resourceFor (controller → ACL resource) -------------------------------------------- + + #[Test] + public function a_default_namespace_controller_maps_to_its_controller_class(): void + { + $this->assertSame('IndexController', $this->plugin()->pubResourceFor($this->req('index'))); + } + + #[Test] + public function a_module_controller_is_prefixed_with_the_studly_module(): void + { + $this->assertSame('Cms_AdminController', $this->plugin()->pubResourceFor($this->req('admin', 'cms'))); + } + + #[Test] + public function hyphenated_controller_and_module_names_studly_case(): void + { + $this->assertSame('MyMod_UserAdminController', $this->plugin()->pubResourceFor($this->req('user-admin', 'my-mod'))); + } + + #[Test] + public function an_empty_controller_name_has_no_resource(): void + { + $this->assertNull($this->plugin()->pubResourceFor($this->req(''))); + } +} diff --git a/tests/Integration/SavepointIsolationTest.php b/tests/Integration/SavepointIsolationTest.php new file mode 100644 index 0000000..93f0910 --- /dev/null +++ b/tests/Integration/SavepointIsolationTest.php @@ -0,0 +1,121 @@ +insert([ + 'org_id' => 'sp-org', 'filename' => $key, 'mime_type' => 'text/plain', + 'storage_key' => $key, 'disk' => 'local', 'kind' => 'file', + ]); + } + + private function markerCount(string $key): int + { + return (int) $this->db->fetchOne('SELECT COUNT(*) FROM media WHERE filename = ?', [$key]); + } + + #[Test] + public function a_nested_begin_commit_does_not_throw_and_persists_within_the_outer_txn(): void + { + // Under the stock adapter this beginTransaction() (inside the base outer txn) would throw. + $this->db->beginTransaction(); // depth 1 → SAVEPOINT tiger_sp_1 + $this->insertMarker('sp-nest-commit'); + $this->db->commit(); // → RELEASE SAVEPOINT tiger_sp_1 (no error) + + $this->assertSame(1, $this->markerCount('sp-nest-commit'), 'the inner-committed row is visible'); + } + + #[Test] + public function a_nested_rollback_undoes_only_its_own_work(): void + { + $this->insertMarker('sp-outer'); // outer-txn work + $this->db->beginTransaction(); // SAVEPOINT + $this->insertMarker('sp-inner'); + $this->db->rollBack(); // → ROLLBACK TO SAVEPOINT (inner only) + + $this->assertSame(1, $this->markerCount('sp-outer'), 'outer work survives the inner rollback'); + $this->assertSame(0, $this->markerCount('sp-inner'), 'inner work is undone'); + } + + #[Test] + public function three_levels_of_nesting_compose(): void + { + $this->db->beginTransaction(); // depth 1 + $this->db->beginTransaction(); // depth 2 + $this->insertMarker('sp-deep'); + $this->db->commit(); // release depth 2 + $this->db->commit(); // release depth 1 + + $this->assertSame(1, $this->markerCount('sp-deep'), 'a two-deep nest resolves cleanly'); + } + + // The two probes below are identical on purpose: each asserts a clean start THEN inner-commits the + // SAME marker. If the outer per-test rollback failed to discard an inner-committed row, whichever + // runs second would see count>0 at the top and fail — so both passing proves isolation holds across + // tests despite an inner COMMIT, in any order. + + #[Test] + public function isolation_survives_an_inner_commit_probe_a(): void + { + $this->isolationProbe(); + } + + #[Test] + public function isolation_survives_an_inner_commit_probe_b(): void + { + $this->isolationProbe(); + } + + private function isolationProbe(): void + { + $this->assertSame(0, $this->markerCount('sp-iso'), 'each test starts clean — the prior outer rollback discarded even inner-committed savepoint work'); + $this->db->beginTransaction(); + $this->insertMarker('sp-iso'); + $this->db->commit(); + $this->assertSame(1, $this->markerCount('sp-iso')); + } + + #[Test] + public function a_real_service_transaction_now_nests_inside_the_per_test_txn(): void + { + // The payoff: dispatch a real /api service whose save() opens its OWN _transaction(), WITHOUT + // escaping the base txn first (the Wave-3 workaround). It must commit cleanly and the row is then + // discarded by the base rollback — service happy-paths test with true rollback isolation. + $this->loginAs('admin'); + Zend_Registry::set('tiger.auth.stateless', true); // no CSRF session in CLI + Zend_Registry::set('Zend_Config', new Zend_Config(['tiger' => ['i18n' => ['locales' => 'en,es']]], true)); + + $res = (new Access_Service_User([ + 'action' => 'save', 'email' => 'savepoint@w3ctest.com', 'username' => 'spuser', 'status' => 'active', + ]))->getResponse(); + + $this->assertSame(1, (int) $res->result, 'the service save committed its own nested transaction'); + $this->assertSame(1, (int) $this->db->fetchOne('SELECT COUNT(*) FROM user WHERE email = ?', ['savepoint@w3ctest.com']), 'the row landed and is visible in-test'); + + $reg = Zend_Registry::getInstance(); + if ($reg->offsetExists('tiger.auth.stateless')) { $reg->offsetUnset('tiger.auth.stateless'); } + } +} diff --git a/tests/Support/IntegrationTestCase.php b/tests/Support/IntegrationTestCase.php index 2b3a598..7a6d658 100644 --- a/tests/Support/IntegrationTestCase.php +++ b/tests/Support/IntegrationTestCase.php @@ -121,7 +121,9 @@ protected function logout(): void private static function adapter(string $name): Zend_Db_Adapter_Abstract { if (self::$sharedDb === null) { - self::$sharedDb = Zend_Db::factory('Pdo_Mysql', [ + // The savepoint-aware adapter (not the stock one) so a service's own `_transaction()` nests + // inside the per-test outer transaction instead of throwing — see SavepointAdapter. + self::$sharedDb = new SavepointAdapter([ 'host' => getenv('TIGER_TEST_DB_HOST') ?: '127.0.0.1', 'port' => (int) (getenv('TIGER_TEST_DB_PORT') ?: 3306), 'dbname' => $name, diff --git a/tests/Support/SavepointAdapter.php b/tests/Support/SavepointAdapter.php new file mode 100644 index 0000000..d94dcaf --- /dev/null +++ b/tests/Support/SavepointAdapter.php @@ -0,0 +1,76 @@ +_txDepth === 0) { + parent::_beginTransaction(); + } else { + $this->_connect(); + $this->_connection->exec('SAVEPOINT ' . $this->_savepointName($this->_txDepth)); + } + $this->_txDepth++; + } + + /** Commit: RELEASE the inner SAVEPOINT, or really COMMIT when unwinding the outermost level. */ + protected function _commit() + { + if ($this->_txDepth <= 1) { + parent::_commit(); + $this->_txDepth = 0; + return; + } + $this->_txDepth--; + $this->_connect(); + $this->_connection->exec('RELEASE SAVEPOINT ' . $this->_savepointName($this->_txDepth)); + } + + /** Roll back: ROLLBACK TO the inner SAVEPOINT, or really ROLLBACK the outermost level. */ + protected function _rollBack() + { + if ($this->_txDepth <= 1) { + parent::_rollBack(); + $this->_txDepth = 0; + return; + } + $this->_txDepth--; + $this->_connect(); + $this->_connection->exec('ROLLBACK TO SAVEPOINT ' . $this->_savepointName($this->_txDepth)); + } + + /** A savepoint identifier for a given nesting level (LIFO, so the level number is unique enough). */ + private function _savepointName(int $level): string + { + return 'tiger_sp_' . $level; + } +} From d92582ff5f02fe9b4693dbdd89a28a6909a26dff Mon Sep 17 00:00:00 2001 From: "Beau Beauchamp, WebTigers" Date: Fri, 24 Jul 2026 14:38:21 -0400 Subject: [PATCH 2/2] test: order-insensitive assertion in TranslationTest::get_for_locale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getForLocale() returns an unordered key=>value map (no ORDER BY), so row order is DB-defined and varies across engines/runs — assertSame (order-sensitive on arrays) flaked in CI's combined unit+integration coverage run. Use assertEquals (same pairs, any order). Surfaced by the new coverage job running both suites in one process. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/Integration/Model/TranslationTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/Integration/Model/TranslationTest.php b/tests/Integration/Model/TranslationTest.php index 68fdfc2..9698859 100644 --- a/tests/Integration/Model/TranslationTest.php +++ b/tests/Integration/Model/TranslationTest.php @@ -96,7 +96,9 @@ public function get_for_locale_returns_only_the_scoped_map(): void $map = $this->t->getForLocale('en', Tiger_Model_Translation::SCOPE_GLOBAL, ''); - $this->assertSame(['a.one' => 'One', 'a.two' => 'Two'], $map, 'exactly the en/global overrides as a key=>value map — no other locale or scope'); + // assertEquals (not assertSame): the map is an unordered key=>value set and getForLocale has no + // ORDER BY, so row order is DB-defined and differs across engines/runs — only the pairs matter. + $this->assertEquals(['a.one' => 'One', 'a.two' => 'Two'], $map, 'exactly the en/global overrides as a key=>value map — no other locale or scope'); } #[Test]