chore: Align MyDash with template + hydra ADRs#34
Merged
Conversation
This was referenced Apr 24, 2026
Contributor
Quality Report — ConductionNL/mydash @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ❌ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ❌ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 103/103 | |||
| npm | ✅ | ✅ 342/342 | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-04-24 10:50 UTC
Download the full PDF report from the workflow artifacts.
`ResponseHelper::error()` was returning `\$exception->getMessage()` in the response body, which surfaces internal error detail (stack traces, DB errors, file paths) to any authenticated caller that triggered a thrown exception. ADR-005 Rule 5 forbids this: "No stack traces in API responses; generic messages." Fix closes the leak immediately by returning a generic \`"Operation failed"\` message to the client regardless of the underlying exception. The helper now accepts two optional parameters for callers that want to preserve visibility + customise the client message: - \`?LoggerInterface \$logger\` — when provided, the real exception is recorded at ERROR level with full context (`['exception' => \$exception]`) before the response is returned. - \`string \$message='Operation failed'\` — generic client-facing text. Signature stays backwards-compatible: all 20 existing \`ResponseHelper::error(exception: \$e)\` call sites across DashboardApiController, TileApiController, WidgetApiController, RuleApiController, AdminController continue to work unchanged, and they now return generic text instead of leaking. Follow-up (not in this commit): thread \`\$this->logger\` through the 6 controllers that currently don't inject a LoggerInterface so the server log regains exception visibility. Tracked in docs/adr-audit.md.
…(ADR-014) Every PHP file in lib/ and tests/Unit/ carried TWO licence declarations that disagreed: the PHPDoc `@license EUPL-1.2` (correct, Conduction standard) plus a separate SPDX block declaring `SPDX-License-Identifier: AGPL-3.0-or-later` with `SPDX-FileCopyrightText: 2024 MyDash Contributors`. Any REUSE-based compliance tooling would reject this — two licences on one file is ambiguous. Removed the SPDX block from all 72 files. The PHPDoc `@license EUPL-1.2` + `@copyright 2024 Conduction b.v.` tags are now the single source of truth. REUSE compliance will be restored by the REUSE.toml addition in the next commit — path-based annotations map **/*.php to EUPL-1.2 + Conduction copyright. Verified: `grep -rl "SPDX-License-Identifier: AGPL" lib/ tests/` returns zero. Same for `SPDX-FileCopyrightText: 2024 MyDash Contributors`. The PHPDoc blocks now terminate cleanly after `@link` / `@license`, no trailing orphan SPDX tags.
ADR-004 requires Conduction apps to route all Nextcloud Vue components through the @conduction/nextcloud-vue wrapper instead of importing from @nextcloud/vue directly. Nine Vue files had direct imports: - src/components/WidgetWrapper.vue (NcButton) - src/components/TileEditor.vue (NcModal, NcButton, NcTextField, NcSelect, NcColorPicker) - src/components/WidgetPicker.vue (NcButton, NcTextField, NcEmptyContent) - src/views/Views.vue (NcButton, NcEmptyContent) - src/components/TileCard.vue (NcButton) - src/components/DashboardSwitcher.vue (NcSelect) - src/components/WidgetRenderer.vue (NcDashboardWidget, NcEmptyContent, NcLoadingIcon) - src/components/WidgetStyleEditor.vue (NcModal, NcButton, NcTextField, NcSelect, NcColorPicker, NcCheckboxRadioSwitch) - src/components/admin/AdminSettings.vue (NcButton, NcSelect, NcSelectTags, NcTextField, NcCheckboxRadioSwitch, NcEmptyContent, NcModal) All 9 now import from @conduction/nextcloud-vue. The package was already in package.json dependencies, so no dep bump needed. Verified: `grep -rn "from '@nextcloud/vue'" src/` returns zero matches.
**REUSE.toml**: closes the REUSE-compliance hole from the previous commit. The SPDX block was removed from 72 PHP files; this declaration restores machine-readable licence + copyright data via path-based annotations: - `**/*.php` → EUPL-1.2 + "2024 Conduction B.V." (matches every PHP file's PHPDoc @license/@copyright tags — reuse lint accepts the PHPDoc form when the REUSE.toml declares it explicitly) - Vue / JS / TS / CSS / SCSS / shell → EUPL-1.2 + Conduction - Config / data / markdown / JSON / YAML / XML / TOML / img / l10n / lockfiles → CC0-1.0 (standard Nextcloud-app treatment) Template-matching shape — same structure as nextcloud-app-template's REUSE.toml, year bumped to 2024 (MyDash inception) instead of 2026, and `**/*.toml` added to the CC0 block so REUSE.toml itself is covered. **info.xml**: Nextcloud max-version 33 → 34. Template supports 34; no reason MyDash shouldn't. Skipped the Makefile dev-link symlink helper from the template — the template needs it because the repo is cloned as `nextcloud-app-template` but the <id> is `app-template`. MyDash's <id> is `mydash` and the repo is cloned as `mydash`, so the symlink trick is a no-op here.
Three new markdown docs wire into the Docusaurus site (autogenerated sidebar picks them up automatically): - `docs/architecture.md` — component map (Controller/Service/Mapper layers with concrete class counts), request flow for dashboard update, auth posture, DI posture, capability table mapping to openspec/specs/, and an explicit "does NOT do" list covering the ADRs that are legitimately N/A for MyDash. - `docs/adr/README.md` — names the canonical ADR location (ConductionNL/hydra/openspec/architecture/) and explains why app repos no longer carry stale copies (decidesk #71 false-positive drift lesson). Quick index of the 23 ADRs. - `docs/adr-audit.md` — per-ADR compliance matrix for MyDash as of 2026-04-24. 25 compliant / 4 partial / 2 gaps / 14 N/A. Each row carries a status + evidence + link to fix when applicable. Captures the verified findings from the two-agent audit run at the start of this PR: - 0 service-locator escapes in lib/ (fixed in commit 5712903) - `ResponseHelper::error` no longer leaks (commit 9be1255) - 72 PHP files have clean EUPL-1.2 headers (commit ab9fc70) - 9 @nextcloud/vue imports swapped (commit 222cf3f) - REUSE.toml + max-version bump (commit b5ca3a4) The two remaining `❌` gaps — @SPEC annotation pass + Newman collection — are flagged for OpenSpec changes + Hydra pickup in the next commit. Corrects the earlier audit's false-alarm IDOR claim (agent 2 stopped at controller null-check, didn't trace through to the service-layer ownership verification). ADR-005 per-object auth is actually PASS once you follow DashboardApiController::update → PermissionService → DashboardService's ownership guard.
Converts the two ❌ items from docs/adr-audit.md into formal OpenSpec changes so Hydra's pipeline can pick them up once this PR merges. **spec-annotation-pass-2026-04-24/** — Close the ADR-003 `@spec` tag gap. Runs `/opsx-annotate mydash` against the 66 methods already classified by `openspec/coverage-report.md` (61 Bucket 1 + 5 Bucket 2b from the legacy-widget-bridge retrofit). Design.md resolves the 3 NEEDS-REVIEW flags from the coverage scan: - `DashboardResolver::getEffectivePermissionLevel` vs `PermissionService::getEffectivePermissionLevel` — both tag with `@spec permissions/spec.md#requirement-effective-permission-level`; PermissionService is the authoritative impl, DashboardResolver is a thin delegator. - `MyDashAdmin::getForm` + `MyDashAdminSection::getID` — treat as plumbing (Nextcloud admin-UI boilerplate, no domain behaviour), skip `@spec` annotation with an explanatory docblock note. **newman-integration-suite-2026-04-24/** — Close the ADR-008 Newman gap. Adds `tests/integration/mydash.postman_collection.json` covering all 17 OCS endpoints across 6 folders (Health+Metrics, Dashboards, Tiles, Widgets, Rules, Admin). Fixture setup/teardown pattern avoids test pollution; admin-vs-member auth branch tests verify authorization posture. `.github/workflows/code-quality.yml` already has `enable-newman: true` — the collection is all that's missing. Each change has proposal.md + design.md + tasks.md + specs/README.md explaining that they're annotation-only / test-only (no Requirement deltas needed). Format mirrors the archived retrofits in openspec/changes/archive/. GitHub issues + Hydra dispatch handled in a follow-up — spec files land on development first so Hydra can locate them.
rubenvdlinde
force-pushed
the
feature/template-and-adr-cleanup
branch
from
aaf1b2b to
462587d
Compare
Contributor
Quality Report — ConductionNL/mydash @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ❌ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ❌ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ✅ | ✅ 342/342 | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-04-30 21:16 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
added a commit
that referenced
this pull request
Apr 30, 2026
…/nextcloud-vue (ADR-004) (#71) Two stragglers added by widget-add-edit-modal (#65/#68 runtime-shell) and widget-context-menu (#60) before ADR-004 landed via #34. Swap is mechanical: NcButton + NcEmptyContent are already exported by @conduction/nextcloud-vue and used elsewhere in the codebase (TileCard, WidgetRenderer, AdminSettings).
rubenvdlinde
added a commit
that referenced
this pull request
May 3, 2026
* fix(security): Stop leaking exception messages to API clients (ADR-005) `ResponseHelper::error()` was returning `\$exception->getMessage()` in the response body, which surfaces internal error detail (stack traces, DB errors, file paths) to any authenticated caller that triggered a thrown exception. ADR-005 Rule 5 forbids this: "No stack traces in API responses; generic messages." Fix closes the leak immediately by returning a generic \`"Operation failed"\` message to the client regardless of the underlying exception. The helper now accepts two optional parameters for callers that want to preserve visibility + customise the client message: - \`?LoggerInterface \$logger\` — when provided, the real exception is recorded at ERROR level with full context (`['exception' => \$exception]`) before the response is returned. - \`string \$message='Operation failed'\` — generic client-facing text. Signature stays backwards-compatible: all 20 existing \`ResponseHelper::error(exception: \$e)\` call sites across DashboardApiController, TileApiController, WidgetApiController, RuleApiController, AdminController continue to work unchanged, and they now return generic text instead of leaking. Follow-up (not in this commit): thread \`\$this->logger\` through the 6 controllers that currently don't inject a LoggerInterface so the server log regains exception visibility. Tracked in docs/adr-audit.md. * fix(licence): Remove contradictory AGPL SPDX block from 72 PHP files (ADR-014) Every PHP file in lib/ and tests/Unit/ carried TWO licence declarations that disagreed: the PHPDoc `@license EUPL-1.2` (correct, Conduction standard) plus a separate SPDX block declaring `SPDX-License-Identifier: AGPL-3.0-or-later` with `SPDX-FileCopyrightText: 2024 MyDash Contributors`. Any REUSE-based compliance tooling would reject this — two licences on one file is ambiguous. Removed the SPDX block from all 72 files. The PHPDoc `@license EUPL-1.2` + `@copyright 2024 Conduction b.v.` tags are now the single source of truth. REUSE compliance will be restored by the REUSE.toml addition in the next commit — path-based annotations map **/*.php to EUPL-1.2 + Conduction copyright. Verified: `grep -rl "SPDX-License-Identifier: AGPL" lib/ tests/` returns zero. Same for `SPDX-FileCopyrightText: 2024 MyDash Contributors`. The PHPDoc blocks now terminate cleanly after `@link` / `@license`, no trailing orphan SPDX tags. * refactor(frontend): @nextcloud/vue → @conduction/nextcloud-vue (ADR-004) ADR-004 requires Conduction apps to route all Nextcloud Vue components through the @conduction/nextcloud-vue wrapper instead of importing from @nextcloud/vue directly. Nine Vue files had direct imports: - src/components/WidgetWrapper.vue (NcButton) - src/components/TileEditor.vue (NcModal, NcButton, NcTextField, NcSelect, NcColorPicker) - src/components/WidgetPicker.vue (NcButton, NcTextField, NcEmptyContent) - src/views/Views.vue (NcButton, NcEmptyContent) - src/components/TileCard.vue (NcButton) - src/components/DashboardSwitcher.vue (NcSelect) - src/components/WidgetRenderer.vue (NcDashboardWidget, NcEmptyContent, NcLoadingIcon) - src/components/WidgetStyleEditor.vue (NcModal, NcButton, NcTextField, NcSelect, NcColorPicker, NcCheckboxRadioSwitch) - src/components/admin/AdminSettings.vue (NcButton, NcSelect, NcSelectTags, NcTextField, NcCheckboxRadioSwitch, NcEmptyContent, NcModal) All 9 now import from @conduction/nextcloud-vue. The package was already in package.json dependencies, so no dep bump needed. Verified: `grep -rn "from '@nextcloud/vue'" src/` returns zero matches. * chore: Add REUSE.toml + bump Nextcloud max-version to 34 **REUSE.toml**: closes the REUSE-compliance hole from the previous commit. The SPDX block was removed from 72 PHP files; this declaration restores machine-readable licence + copyright data via path-based annotations: - `**/*.php` → EUPL-1.2 + "2024 Conduction B.V." (matches every PHP file's PHPDoc @license/@copyright tags — reuse lint accepts the PHPDoc form when the REUSE.toml declares it explicitly) - Vue / JS / TS / CSS / SCSS / shell → EUPL-1.2 + Conduction - Config / data / markdown / JSON / YAML / XML / TOML / img / l10n / lockfiles → CC0-1.0 (standard Nextcloud-app treatment) Template-matching shape — same structure as nextcloud-app-template's REUSE.toml, year bumped to 2024 (MyDash inception) instead of 2026, and `**/*.toml` added to the CC0 block so REUSE.toml itself is covered. **info.xml**: Nextcloud max-version 33 → 34. Template supports 34; no reason MyDash shouldn't. Skipped the Makefile dev-link symlink helper from the template — the template needs it because the repo is cloned as `nextcloud-app-template` but the <id> is `app-template`. MyDash's <id> is `mydash` and the repo is cloned as `mydash`, so the symlink trick is a no-op here. * docs: Add architecture, ADR index, ADR compliance audit Three new markdown docs wire into the Docusaurus site (autogenerated sidebar picks them up automatically): - `docs/architecture.md` — component map (Controller/Service/Mapper layers with concrete class counts), request flow for dashboard update, auth posture, DI posture, capability table mapping to openspec/specs/, and an explicit "does NOT do" list covering the ADRs that are legitimately N/A for MyDash. - `docs/adr/README.md` — names the canonical ADR location (ConductionNL/hydra/openspec/architecture/) and explains why app repos no longer carry stale copies (decidesk #71 false-positive drift lesson). Quick index of the 23 ADRs. - `docs/adr-audit.md` — per-ADR compliance matrix for MyDash as of 2026-04-24. 25 compliant / 4 partial / 2 gaps / 14 N/A. Each row carries a status + evidence + link to fix when applicable. Captures the verified findings from the two-agent audit run at the start of this PR: - 0 service-locator escapes in lib/ (fixed in commit 5712903) - `ResponseHelper::error` no longer leaks (commit 9be1255) - 72 PHP files have clean EUPL-1.2 headers (commit ab9fc70) - 9 @nextcloud/vue imports swapped (commit 222cf3f) - REUSE.toml + max-version bump (commit b5ca3a4) The two remaining `❌` gaps — @SPEC annotation pass + Newman collection — are flagged for OpenSpec changes + Hydra pickup in the next commit. Corrects the earlier audit's false-alarm IDOR claim (agent 2 stopped at controller null-check, didn't trace through to the service-layer ownership verification). ADR-005 per-object auth is actually PASS once you follow DashboardApiController::update → PermissionService → DashboardService's ownership guard. * spec: Two OpenSpec changes for the remaining ADR gaps Converts the two ❌ items from docs/adr-audit.md into formal OpenSpec changes so Hydra's pipeline can pick them up once this PR merges. **spec-annotation-pass-2026-04-24/** — Close the ADR-003 `@spec` tag gap. Runs `/opsx-annotate mydash` against the 66 methods already classified by `openspec/coverage-report.md` (61 Bucket 1 + 5 Bucket 2b from the legacy-widget-bridge retrofit). Design.md resolves the 3 NEEDS-REVIEW flags from the coverage scan: - `DashboardResolver::getEffectivePermissionLevel` vs `PermissionService::getEffectivePermissionLevel` — both tag with `@spec permissions/spec.md#requirement-effective-permission-level`; PermissionService is the authoritative impl, DashboardResolver is a thin delegator. - `MyDashAdmin::getForm` + `MyDashAdminSection::getID` — treat as plumbing (Nextcloud admin-UI boilerplate, no domain behaviour), skip `@spec` annotation with an explanatory docblock note. **newman-integration-suite-2026-04-24/** — Close the ADR-008 Newman gap. Adds `tests/integration/mydash.postman_collection.json` covering all 17 OCS endpoints across 6 folders (Health+Metrics, Dashboards, Tiles, Widgets, Rules, Admin). Fixture setup/teardown pattern avoids test pollution; admin-vs-member auth branch tests verify authorization posture. `.github/workflows/code-quality.yml` already has `enable-newman: true` — the collection is all that's missing. Each change has proposal.md + design.md + tasks.md + specs/README.md explaining that they're annotation-only / test-only (no Requirement deltas needed). Format mirrors the archived retrofits in openspec/changes/archive/. GitHub issues + Hydra dispatch handled in a follow-up — spec files land on development first so Hydra can locate them. * chore: update SBOM --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
rubenvdlinde
added a commit
that referenced
this pull request
May 3, 2026
…/nextcloud-vue (ADR-004) (#71) Two stragglers added by widget-add-edit-modal (#65/#68 runtime-shell) and widget-context-menu (#60) before ADR-004 landed via #34. Swap is mechanical: NcButton + NcEmptyContent are already exported by @conduction/nextcloud-vue and used elsewhere in the codebase (TileCard, WidgetRenderer, AdminSettings).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Template + ADR audit pass on MyDash. Consolidates 6 in-flight feature PRs into
developmentfirst (via admin-merge), then applies focused fixes for the real gaps the audit surfaced.No mega-refactor — MyDash was already mostly template-aligned (full QA tooling, 9 Conduction workflows, complete i18n, Docusaurus, 10 archived openspec changes). This PR closes the specific findings, not the whole 26-commit template cycle done for app-versions.
Pre-PR: in-flight consolidation
Admin-merged into
developmentbefore starting:Closed #20 (chore/19/harmonize-gitignore) as superseded by #13 + #14.
Commits in this PR (7 of them)
refactor(PageController): Inject IDashboardManager via constructor (ADR-003)\OC::$server->get()escape inlib/. Constructor DI now consistent across every controller + service.fix(security): Stop leaking exception messages to API clients (ADR-005)ResponseHelper::error()was returning$exception->getMessage()to the client — stack-trace / internal-error leakage. Now returns generic'Operation failed'. Signature extended with optional?LoggerInterface $logger+string $messageso callers keep server-side visibility. 20 call sites unchanged.fix(licence): Remove contradictory AGPL SPDX block from 72 PHP files (ADR-014)@license EUPL-1.2(PHPDoc) ANDSPDX-License-Identifier: AGPL-3.0-or-later(SPDX block). Removed the AGPL block; PHPDoc + REUSE.toml (next commit) are now the single source.refactor(frontend): @nextcloud/vue → @conduction/nextcloud-vue (ADR-004)@nextcloud/vueimports. All route through@conduction/nextcloud-vuenow.chore: Add REUSE.toml + bump Nextcloud max-version to 34docs: Add architecture, ADR index, ADR compliance auditarchitecture.md(component map, request flow, auth + DI posture),adr/README.md(ADR index + why app repos don't duplicate hydra),adr-audit.md(per-ADR compliance matrix: 25 ✅ / 4spec: Two OpenSpec changes for the remaining ADR gapsopenspec/changes/entries Hydra can pick up:spec-annotation-pass-2026-04-24(66-method@spectag pass) +newman-integration-suite-2026-04-24(17-endpoint Postman collection).ADRs addressed
\OC::$server->get()escape@nextcloud/vueimports@conduction/nextcloud-vue$e->getMessage()returned to clientFull matrix in
docs/adr-audit.md.Follow-ups (filed separately as openspec changes)
@specannotation pass — 66 methods × 9 capabilities. The coverage scan already did the classification./opsx-annotate mydash+ the 3 NEEDS-REVIEW decisions (design.md spells them out).tests/integration/mydash.postman_collection.jsoncovering all 17 endpoints with fixture setup/teardown pattern.Both will be opened as GitHub issues after this PR merges so Hydra can dispatch.
Audit false-alarm correction
The two-agent audit initially surfaced an IDOR claim on
DashboardApiController::update(). Verified false — agent 2 stopped at the controller-level$this->userId !== nullnull-check and missed$this->permissionService->canEditDashboard(userId, dashboardId)right after. Also traced through to the service layer:DashboardService::deleteDashboardchecks$dashboard->getUserId() !== $userIdand throws'Access denied'before any mutation. Ownership is enforced consistently — audit updated to reflect.Test plan
composer installsucceedscomposer check:strictgreen (phpcs + phpmd + phpstan + psalm + phpunit)npm install && npm run buildsucceeds"Operation failed")@conduction/nextcloud-vuebeing installed)reuse lintaccepts the project