From 247bfcddc7071b0c46751c140f55d78dd7fb04ad Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Sun, 16 Aug 2026 14:03:01 +0200 Subject: [PATCH 1/2] docs(security): waive gate-7 on the two endpoints that address no object gate-7 reports both of these as NoAdminRequired with no recognised guard. An IDOR needs an attacker-supplied identifier, and neither method accepts one: VisibilityPreviewController::preview takes a candidate rule set and an evaluation context from the request body, loads nothing by id, persists nothing, and evaluates against $this->userId. DashboardShareApiController::searchSharees takes a search string, reads no dashboard and no share, and returns the same directory the core share picker already exposes to any authenticated user. Both null-check $this->userId. searchSharees additionally excludes the caller from its own results, blocks single-character sweeps as a directory enumeration guard, and bounds both searches to 10 rows. Using the exemption the gate provides rather than widening its guard patterns: its own comments record that widening 'would let gate-7 clear real IDORs', and that is a worse trade than two documented waivers. The reason is required and stated. Verified the waiver is load-bearing, not decorative: with it, gate-7 reports zero on this repo; displace the docblock and the finding comes straight back. --- lib/Controller/DashboardShareApiController.php | 7 +++++++ lib/Controller/VisibilityPreviewController.php | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/lib/Controller/DashboardShareApiController.php b/lib/Controller/DashboardShareApiController.php index 8d3145ef..587a8fc1 100644 --- a/lib/Controller/DashboardShareApiController.php +++ b/lib/Controller/DashboardShareApiController.php @@ -300,6 +300,13 @@ public function revokeForRecipient( * @return DataResponse The matching users and groups. * * @spec openspec/specs/dashboard-sharing/spec.md + * + * @no-admin-idor-exempt no object is addressed. The only parameter is a + * search string; the method reads no dashboard and no share, and returns + * the same directory any authenticated user already sees through the core + * share picker. It null-checks `$this->userId`, excludes the caller from + * its own results, blocks single-character sweeps as a directory + * enumeration guard, and bounds both searches to 10 rows. */ #[NoAdminRequired] public function searchSharees(string $query = ''): DataResponse { diff --git a/lib/Controller/VisibilityPreviewController.php b/lib/Controller/VisibilityPreviewController.php index 6e4f98fa..2be7e008 100644 --- a/lib/Controller/VisibilityPreviewController.php +++ b/lib/Controller/VisibilityPreviewController.php @@ -90,6 +90,13 @@ public function __construct( * fail validation. * * @spec openspec/specs/conditional-visibility-editor/spec.md#requirement-req-cvui-005-preview-endpoint-reuses-the-render-time-evaluation-path-and-never-persists + * + * @no-admin-idor-exempt no object is addressed. Both parameters are the + * candidate rule set and an evaluation context taken from the request + * body; the method loads nothing by id, persists nothing, and evaluates + * against `$this->userId` — which it null-checks — so there is no other + * user's object for a caller to reach. An IDOR needs an attacker-supplied + * identifier, and this endpoint accepts none. */ #[NoAdminRequired] public function preview( From ef50f7da958cc92bfcc79fd2b2c1b6a436a21b7c Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Sun, 16 Aug 2026 14:59:53 +0200 Subject: [PATCH 2/2] fix(gates): clear gate-26, gate-35 and gate-57 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gate-26 — TemplatesPage.vue was reported as having no visual proof while tests/e2e/admin-templates-page.spec.ts tests exactly that page. The gate matches a page against its component stem, the spec only ever wrote 'TemplatesPage' in comments, and the corpus strips comments (a comment is not a baseline, .github#358). The selector is now a named constant, so the link between spec and component is executable rather than prose. Verified load-bearing: rename the constant and the finding returns. gate-57 — DashboardVersionService::deleteVersionsForDashboard() had no production caller anywhere. VersionsListener IS registered on DashboardDeletedEvent and does the same single mapper call directly, so the cascade has always worked and the service method was dead. Removed. The interesting part is what the coverage was doing: the dead wrapper had a unit test, and VersionsListener — the live path — had none. So the tested path was the one nothing called. VersionsListener now has its own test, including that an unrelated event deletes nothing (without which the listener could fire on any event and the happy-path test would still pass) and that a mapper failure is logged rather than rethrown, which REQ-CSC-006 requires because a throw would abort the sibling listeners on the same event. gate-35 — the inert news card's thumbnail now says aria-hidden="true" alongside alt="". The headline beside it carries the meaning and the card is inert, so there is no link for the image to name; the attribute states that explicitly instead of leaving alt="" to imply it. gate-37 (aria-hidden-focusable) re-checked and still clear — an img is not focusable. --- lib/Service/DashboardVersionService.php | 18 --- .../Widgets/Renderers/NewsWidget.vue | 7 +- tests/Unit/Listener/VersionsListenerTest.php | 150 ++++++++++++++++++ .../Service/DashboardVersionServiceTest.php | 19 +-- tests/e2e/admin-templates-page.spec.ts | 19 ++- 5 files changed, 175 insertions(+), 38 deletions(-) create mode 100644 tests/Unit/Listener/VersionsListenerTest.php diff --git a/lib/Service/DashboardVersionService.php b/lib/Service/DashboardVersionService.php index d0d6138c..a853f9e7 100644 --- a/lib/Service/DashboardVersionService.php +++ b/lib/Service/DashboardVersionService.php @@ -527,24 +527,6 @@ public function restoreVersion( ]; }//end restoreVersion() - /** - * Cascade-delete every snapshot row for a dashboard. - * - * Designed to be called from the dashboard delete path or, in the - * future, the cascade-events VersionsListener stub. Idempotent. - * - * @param string $dashboardUuid The dashboard UUID. - * - * @return integer The number of rows deleted. - * - * @spec openspec/specs/dashboard-versioning/spec.md - */ - public function deleteVersionsForDashboard(string $dashboardUuid): int { - return $this->versionMapper->deleteByDashboardUuid( - dashboardUuid: $dashboardUuid - ); - }//end deleteVersionsForDashboard() - /** * Whether the supplied dashboard is groupfolder-backed * (REQ-VERS-008). Currently always false because the groupfolder diff --git a/src/components/Widgets/Renderers/NewsWidget.vue b/src/components/Widgets/Renderers/NewsWidget.vue index 24cf65eb..86b50fc1 100644 --- a/src/components/Widgets/Renderers/NewsWidget.vue +++ b/src/components/Widgets/Renderers/NewsWidget.vue @@ -74,11 +74,16 @@
+ + alt="" + aria-hidden="true" />

{{ item.title }} diff --git a/tests/Unit/Listener/VersionsListenerTest.php b/tests/Unit/Listener/VersionsListenerTest.php new file mode 100644 index 00000000..192d6daa --- /dev/null +++ b/tests/Unit/Listener/VersionsListenerTest.php @@ -0,0 +1,150 @@ + + * @copyright 2026 Conduction b.v. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * @link https://conduction.nl + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace OCA\LaunchPad\Tests\Unit\Listener; + +use DateTimeImmutable; +use OCA\LaunchPad\Db\DashboardVersionMapper; +use OCA\LaunchPad\Event\DashboardDeletedEvent; +use OCA\LaunchPad\Listener\VersionsListener; +use OCP\EventDispatcher\Event; +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; +use RuntimeException; + +/** + * @covers \OCA\LaunchPad\Listener\VersionsListener + */ +class VersionsListenerTest extends TestCase { + /** + * The version row mapper. + * + * @var DashboardVersionMapper|MockObject + */ + private DashboardVersionMapper|MockObject $versionMapper; + + /** + * The logger. + * + * @var LoggerInterface|MockObject + */ + private LoggerInterface|MockObject $logger; + + /** + * The listener under test. + * + * @var VersionsListener + */ + private VersionsListener $listener; + + /** + * Build the listener with mocked collaborators. + * + * @return void + */ + protected function setUp(): void { + $this->versionMapper = $this->createMock(DashboardVersionMapper::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->listener = new VersionsListener( + versionMapper: $this->versionMapper, + logger: $this->logger + ); + }//end setUp() + + /** + * Build a DashboardDeletedEvent for the given uuid. + * + * @param string $uuid The deleted dashboard's uuid. + * + * @return DashboardDeletedEvent + */ + private function event(string $uuid): DashboardDeletedEvent { + return new DashboardDeletedEvent( + dashboardUuid: $uuid, + ownerUserId: 'alice', + type: 'personal', + deletedAt: new DateTimeImmutable('2026-08-16T12:00:00+00:00') + ); + }//end event() + + /** + * The snapshots of a deleted dashboard are removed, scoped to that + * dashboard's uuid. + * + * @return void + * + * @spec openspec/specs/dashboard-cascade-events/spec.md + */ + public function testDeletedDashboardHasItsVersionRowsRemoved(): void { + $this->versionMapper->expects($this->once()) + ->method('deleteByDashboardUuid') + ->with(dashboardUuid: 'dash-1') + ->willReturn(7); + + $this->listener->handle($this->event('dash-1')); + }//end testDeletedDashboardHasItsVersionRowsRemoved() + + /** + * An unrelated event leaves the version rows alone. + * + * Without this the listener could delete on ANY dispatched event and the + * test above would still pass — the instanceof guard is the only thing + * standing between this and a cascade that fires on the wrong signal. + * + * @return void + * + * @spec openspec/specs/dashboard-cascade-events/spec.md + */ + public function testAnUnrelatedEventDeletesNothing(): void { + $this->versionMapper->expects($this->never()) + ->method('deleteByDashboardUuid'); + + $this->listener->handle(new class extends Event { + }); + }//end testAnUnrelatedEventDeletesNothing() + + /** + * A mapper failure is logged and swallowed. + * + * REQ-CSC-006 is log-and-continue: this listener is one of several on + * DashboardDeletedEvent, and a throw here would abort the siblings that + * have not run yet, leaving a half-cascaded delete behind. + * + * @return void + * + * @spec openspec/specs/dashboard-cascade-events/spec.md + */ + public function testAMapperFailureIsLoggedAndNotRethrown(): void { + $this->versionMapper->method('deleteByDashboardUuid') + ->willThrowException(new RuntimeException('database is gone')); + + $this->logger->expects($this->once()) + ->method('warning') + ->with($this->stringContains('database is gone'), $this->anything()); + + $this->listener->handle($this->event('dash-1')); + }//end testAMapperFailureIsLoggedAndNotRethrown() +}//end class diff --git a/tests/Unit/Service/DashboardVersionServiceTest.php b/tests/Unit/Service/DashboardVersionServiceTest.php index b87929ec..5ced1976 100644 --- a/tests/Unit/Service/DashboardVersionServiceTest.php +++ b/tests/Unit/Service/DashboardVersionServiceTest.php @@ -550,19 +550,12 @@ public function testAdminMayRestoreOtherUsersDashboard(): void { * * @return void */ - public function testDeleteVersionsForDashboardDelegates(): void { - $this->versionMapper->expects($this->once()) - ->method('deleteByDashboardUuid') - ->with(dashboardUuid: 'd-uuid-1') - ->willReturn(7); - - $this->assertSame( - 7, - $this->service->deleteVersionsForDashboard( - dashboardUuid: 'd-uuid-1' - ) - ); - }//end testDeleteVersionsForDashboardDelegates() + // The cascade delete is covered where it actually happens, in + // tests/Unit/Listener/VersionsListenerTest.php. DashboardVersionService + // used to carry a deleteVersionsForDashboard() wrapper around the same + // single mapper call; nothing in production ever reached it, because + // VersionsListener — which IS registered on DashboardDeletedEvent — calls + // the mapper directly. This test was the wrapper's only caller anywhere. // ========================================================================= // WF1: restoreVersion transaction envelope (wave-12 regression tests) diff --git a/tests/e2e/admin-templates-page.spec.ts b/tests/e2e/admin-templates-page.spec.ts index 948ed84d..aa40b20b 100644 --- a/tests/e2e/admin-templates-page.spec.ts +++ b/tests/e2e/admin-templates-page.spec.ts @@ -41,6 +41,13 @@ const ADMIN = { const SETTINGS_URL = `${BASE}/index.php/settings/admin/launchpad` +// The component under test, named after the file it covers. The selector is +// unchanged — this only makes the link between spec and component readable in +// executable code rather than only in the prose above. gate-26 matches a page +// against its component stem, and the stem never appeared outside a comment, +// so a page that HAS e2e coverage was reported as having none. +const TemplatesPage = '[data-test="templates-page"]' + test.describe('admin-templates — Templates page', () => { test.beforeEach(async ({ page }) => { // Same authentication shape as the neighbouring admin spec: the @@ -60,7 +67,7 @@ test.describe('admin-templates — Templates page', () => { await expect(page.locator('[data-test="panel-templates"]')).toBeVisible({ timeout: 20_000, }) - const templatesPage = page.locator('[data-test="templates-page"]') + const templatesPage = page.locator(TemplatesPage) await expect(templatesPage).toBeVisible() await expect(templatesPage.getByRole('heading', { level: 3 })).toBeVisible() @@ -73,7 +80,7 @@ test.describe('admin-templates — Templates page', () => { page, }) => { await page.goto(SETTINGS_URL) - await expect(page.locator('[data-test="templates-page"]')).toBeVisible({ + await expect(page.locator(TemplatesPage)).toBeVisible({ timeout: 20_000, }) @@ -86,14 +93,14 @@ test.describe('admin-templates — Templates page', () => { // Dismiss without saving — nothing is persisted by this spec. await page.keyboard.press('Escape') await expect(editor).toBeHidden({ timeout: 10_000 }) - await expect(page.locator('[data-test="templates-page"]')).toBeVisible() + await expect(page.locator(TemplatesPage)).toBeVisible() }) test('leaving the tab unmounts the page and returning re-mounts it', async ({ page, }) => { await page.goto(SETTINGS_URL) - await expect(page.locator('[data-test="templates-page"]')).toBeVisible({ + await expect(page.locator(TemplatesPage)).toBeVisible({ timeout: 20_000, }) @@ -101,12 +108,12 @@ test.describe('admin-templates — Templates page', () => { // remove TemplatesPage from the DOM entirely — that unmount is what // makes its created() hook re-fetch the template list on return. await page.locator('[data-test="tab-group-dashboards"]').click() - await expect(page.locator('[data-test="templates-page"]')).toBeHidden({ + await expect(page.locator(TemplatesPage)).toBeHidden({ timeout: 10_000, }) await page.locator('[data-test="tab-templates"]').click() - await expect(page.locator('[data-test="templates-page"]')).toBeVisible({ + await expect(page.locator(TemplatesPage)).toBeVisible({ timeout: 10_000, }) await expect(