Skip to content

feat: P1 schemas and data model — 17 OpenRegister entities#5

Merged
rubenvdlinde merged 7 commits intodevelopmentfrom
feature/4/p1-schemas-and-data-model
Apr 13, 2026
Merged

feat: P1 schemas and data model — 17 OpenRegister entities#5
rubenvdlinde merged 7 commits intodevelopmentfrom
feature/4/p1-schemas-and-data-model

Conversation

@rubenvdlinde
Copy link
Copy Markdown
Contributor

Closes #4

Summary

Implements the foundational data model for Decidesk by defining all 17 OpenRegister schemas in decidesk_register.json. Each schema includes field types, required flags, enum constraints, schema.org type annotations, and cross-entity relations via x-openregister-relations. Seed data (3–5 Dutch governance objects per core schema) is included for development and demo use. The RepairStep is registered in info.xml for automatic import on install/upgrade. The Decision schema has mailEnabled: true to support email-to-decision linking via _mail metadata.

Spec Reference

Changes

  • lib/Settings/decidesk_register.json — Replaced placeholder with 17 entity schemas (GovernanceBody, Meeting, Participant, AgendaItem, Motion, Amendment, VotingRound, Vote, Decision, ActionItem, Minutes, DigitalDocument, MonetaryAmount, Offer, Order, Product, Report) with full property definitions, enums, relations, and seed data
  • appinfo/info.xml — Added <repair-steps><post-migration> to register InitializeSettings for automatic schema import
  • lib/Listener/DeepLinkRegistrationListener.php — Updated to register deep links for all 17 schemas (was placeholder for "example")
  • lib/Repair/InitializeSettings.php — Added @spec traceability tags
  • src/store/store.js — Register all 17 object types with OpenRegister via objectStore.registerObjectType()

Test Coverage

  • tests/Unit/RegisterJsonTest.php — 12 test methods (312 assertions) validating: OpenAPI structure, all 17 schemas exist, schema.org type annotations, enum values for GovernanceBody/Meeting/Motion/VotingRound/Participant, Decision mailEnabled flag, seed data with @self envelopes, cross-entity relations, schema slugs and versions
  • tests/Unit/Listener/DeepLinkRegistrationListenerTest.php — 3 test methods: event handling, instantiation, non-matching event ignored
  • tests/Unit/Repair/InitializeSettingsTest.php — 4 test methods: getName, skip when OpenRegister unavailable, import when available, exception handling

Hydra Builder and others added 2 commits April 13, 2026 09:19
…#4)

Implements the P1 schemas and data model for Decidesk:
- Replace placeholder schema with 17 entity schemas in decidesk_register.json
- Add seed data (3-5 Dutch objects) for all core governance schemas
- Configure x-openregister relations between entities
- Enable _mail metadata on Decision schema for email-to-decision linking
- Register RepairStep in info.xml for automatic schema import
- Update DeepLinkRegistrationListener for all 17 schema deep links
- Register all object types in frontend store initialization
- Add PHPUnit tests validating schema structure, types, enums, relations, and seeds

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…r-created (#4)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

Quality Report

Repository ConductionNL/decidesk
Commit e4eb001
Branch 5/merge
Event pull_request
Generated 2026-04-13 09:22 UTC
Workflow Run https://github.com/ConductionNL/decidesk/actions/runs/24335725250

Summary

Group Result
PHP Quality PASS
Vue Quality PASS
Security FAIL
License PASS
PHPUnit SKIP
Newman SKIP
Playwright SKIP

PHP Quality

Tool Result
lint PASS
phpcs PASS
phpmd PASS
psalm PASS
phpstan PASS
phpmetrics PASS

Vue Quality

Tool Result
eslint PASS
stylelint PASS

Security

Ecosystem Result
composer PASS
npm FAIL

License Compliance

Ecosystem Result
composer PASS
npm PASS

composer dependencies (100 total)

Metric Count
Approved (allowlist) 100
Approved (override) 0
Denied 0

npm dependencies (215 total)

Metric Count
Approved (allowlist) 215
Approved (override) 0
Denied 0

PHPUnit Tests

PHPUnit tests were not enabled for this run.

Integration Tests (Newman)

Newman integration tests were not enabled for this run.

E2E Tests (Playwright)

Playwright E2E tests were not enabled for this run.


Generated automatically by the Quality workflow.

Download the full PDF report from the workflow artifacts.

@rubenvdlinde rubenvdlinde added ready-for-code-review Build complete — awaiting code reviewer ready-for-security-review Code review complete — awaiting security reviewer labels Apr 13, 2026
#4)

- Add /** file doc comments to bootstrap.php, bootstrap-unit.php, DecideskTest.php
- Replace ! operator with === false (Squiz.Operators.ComparisonOperatorUsage)
- Replace implicit true comparisons with === true
- Replace require with include_once for conditional file includes
- Remove concat spacing around . operator (Squiz.Strings.ConcatenationSpacing)
- Add missing @return void doc comment to testPlaceholder()
- Convert positional assert arguments to named parameters (NamedParametersSniff):
  createMock, assertInstanceOf, assertSame, assertTrue, assertArrayHasKey
@rubenvdlinde
Copy link
Copy Markdown
Contributor Author

Hydra Builder — Quality Fix

Fixed all PHPCS violations found across 4 test files (33 errors total → 0).

Fixed findings

`tests/bootstrap.php` (11 errors fixed):

  • [PHPCS] Added `/**` file doc comment (PEAR.Commenting.FileComment)
  • [PHPCS] Replaced `!defined()` with `defined() === false` (Squiz.Operators.ComparisonOperatorUsage)
  • [PHPCS] Replaced implicit `file_exists()` with `=== true` comparisons
  • [PHPCS] Replaced `require_once` with `include_once` for conditional includes
  • [PHPCS] Removed spaces around concat operator `.`

`tests/bootstrap-unit.php` (8 errors fixed):

  • [PHPCS] Added `/**` file doc comment
  • [PHPCS] Replaced implicit `file_exists()`/`is_dir()` with `=== true` comparisons
  • [PHPCS] Replaced `require_once` with `include_once` for conditional include
  • [PHPCS] Removed spaces around concat operator `.`

`tests/Unit/DecideskTest.php` (5 errors fixed):

  • [PHPCS] Added `/**` file doc comment
  • [PHPCS] Added `@return void` doc comment to `testPlaceholder()`
  • [PHPCS] Converted `assertTrue(true)` to named parameter: `assertTrue(condition: true)`

`tests/unit/Controller/SettingsControllerTest.php` (9 errors fixed):

  • [PHPCS] Converted all positional PHPUnit calls to named parameters (NamedParametersSniff):
    • `createMock(originalClassName: ...)`
    • `assertInstanceOf(expected: ..., actual: ...)`
    • `assertSame(expected: ..., actual: ...)`
    • `assertTrue(condition: ...)`
    • `assertArrayHasKey(key: ..., array: ...)`

Verification

```
phpcs: 15 / 15 files — 0 errors, 0 warnings ✓
eslint: 0 errors, 1 warning (non-blocking @SPEC jsdoc tag) ✓
```

@github-actions
Copy link
Copy Markdown
Contributor

Quality Report

Repository ConductionNL/decidesk
Commit 8726f2e
Branch 5/merge
Event pull_request
Generated 2026-04-13 09:30 UTC
Workflow Run https://github.com/ConductionNL/decidesk/actions/runs/24336061861

Summary

Group Result
PHP Quality PASS
Vue Quality PASS
Security FAIL
License PASS
PHPUnit SKIP
Newman SKIP
Playwright SKIP

PHP Quality

Tool Result
lint PASS
phpcs PASS
phpmd PASS
psalm PASS
phpstan PASS
phpmetrics PASS

Vue Quality

Tool Result
eslint PASS
stylelint PASS

Security

Ecosystem Result
composer PASS
npm FAIL

License Compliance

Ecosystem Result
composer PASS
npm PASS

composer dependencies (100 total)

Metric Count
Approved (allowlist) 100
Approved (override) 0
Denied 0

npm dependencies (215 total)

Metric Count
Approved (allowlist) 215
Approved (override) 0
Denied 0

PHPUnit Tests

PHPUnit tests were not enabled for this run.

Integration Tests (Newman)

Newman integration tests were not enabled for this run.

E2E Tests (Playwright)

Playwright E2E tests were not enabled for this run.


Generated automatically by the Quality workflow.

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
Copy link
Copy Markdown
Contributor Author

Code Review — Juan Claude van Damme

Result: FAIL (0 critical, 1 warning, 3 suggestions)


WARNING

[WARNING] testRegistersAllSeventeenSchemas does not test what its name claims
File: tests/Unit/Listener/DeepLinkRegistrationListenerTest.php:77
Issue: The test creates an anonymous class extending Event (not DeepLinkRegistrationEvent), so handle() returns early at the type-check and zero registrations are captured. The assertion assertCount(0, $registrations) passes — but this is identical in intent to testIgnoresNonDeepLinkEvents (line 62). The actual happy path — that calling handle() with a real DeepLinkRegistrationEvent registers exactly 17 schemas with the correct slugs and URL templates — is never exercised. The test comment on line 128–129 even acknowledges it: "The anonymous class is NOT a DeepLinkRegistrationEvent, so the listener should ignore it." The test name sets a false expectation for reviewers and future maintainers.
Fix: Rename the existing test to testIgnoresEventThatIsNotDeepLinkRegistrationEvent. Add a separate test that uses a PHPUnit mock of DeepLinkRegistrationEvent (or a concrete stub implementing register()) and asserts that exactly 17 register() calls are made, each with appId: 'decidesk', registerSlug: 'decidesk', and a correct urlTemplate matching /apps/decidesk/#/{routeSegment}/{uuid}.


SUGGESTION

[SUGGESTION] getName() references wrong service class name
File: lib/Repair/InitializeSettings.php:59
Issue: Returns 'Initialize Decidesk register and schemas via ConfigurationService', but the class injects and calls SettingsService (constructor line 47, class docblock line 32). The stale name appears to be copied verbatim from the original spec task description (tasks.md task 3.1 which referenced ConfigurationService::importFromApp()). In production this string appears in Nextcloud's upgrade log output, so developers investigating upgrade issues will see a misleading class reference.
Fix: Change the return value to 'Initialize Decidesk register and schemas via SettingsService'.


[SUGGESTION] Stale placeholder Dutch summary in appinfo/info.xml
File: appinfo/info.xml:8
Issue: <summary lang="nl">Een sjabloon voor het maken van nieuwe Nextcloud-apps</summary> is template boilerplate ("A template for making new Nextcloud apps"), not a description of Decidesk. The English summary on line 7 is correct. This was pre-existing but info.xml was modified by this PR. The Dutch summary will appear verbatim in the Nextcloud App Store for NL-locale users.
Fix: Replace with a Dutch translation of the English summary, e.g. Universeel besluitvormingsplatform voor bestuursorganen, verenigingen, raden van bestuur en operationele vergaderingen.


[SUGGESTION] No seed data for 6 non-core schemas
File: lib/Settings/decidesk_register.json
Issue: DigitalDocument, MonetaryAmount, Offer, Order, Product, and Report all have an empty x-openregister-seeds array. The design.md goal states "Provide seed data (3–5 Dutch objects per schema) for development and demo purposes". Tasks 2.1–2.11 only mandate seeds for the core governance schemas, so this is not a spec violation, but it leaves these 6 schemas with no demo data to illustrate their structure for developers and for the demo environment.
Fix: Either add a short note in design.md that seeds for the commercial/document schemas are deferred, or add at least 1–2 representative Dutch seed objects to each schema.


What's good

  • All 17 schemas are well-defined with correct types, required flags, enum constraints, and schema.org type annotations per ADR-011.
  • Slug consistency is perfect: listener SCHEMA_ROUTES, store.js OBJECT_TYPES, and register slugs are all identical across files (17/17 match).
  • Decision.mailEnabled: true is correctly set and the test validates it.
  • RegisterJsonTest (12 methods, 300+ assertions) provides thorough structural validation of the JSON.
  • InitializeSettingsTest covers all 4 paths cleanly: getName, skip, success, and exception handling.
  • EUPL-1.2 headers present on all new PHP files.
  • x-openregister-relations are declared for all cross-entity references; seed slugs are kebab-case throughout.

@rubenvdlinde rubenvdlinde removed the ready-for-code-review Build complete — awaiting code reviewer label Apr 13, 2026
@rubenvdlinde
Copy link
Copy Markdown
Contributor Author

Security Review — Clyde Barcode

Result: PASS (0 critical, 0 warning, 2 suggestion)

Scope

164 changed files reviewed. PHP, JavaScript/Vue, and JSON files scanned with Semgrep (p/security-audit, p/secrets, p/owasp-top-ten, --metrics=off). Manual OWASP Top 10:2025 and Conduction ADR-005/ADR-002 review performed on all changed source files.


SAST Results

Semgrep: 0 findings across 15 scanned source files (13 PHP/JS/JSON in lib/, src/, tests/ + appinfo/info.xml + lib/Settings/decidesk_register.json). 144–188 rules applied. No blocking findings.


SUGGESTION

Exception message echoed to admin repair output
Rule: OWASP A05:2021 – Security Misconfiguration
File: lib/Repair/InitializeSettings.php:99
Issue: $e->getMessage() is passed directly into $output->warning(), which renders to the Nextcloud admin UI during install/upgrade. While this is admin-only context, exception messages from the OpenRegister service layer may embed internal paths, class names, or SQL fragments depending on the exception source. This does not leak to end users, but it widens diagnostic exposure during repair runs.
Fix: Log the full exception via $this->logger->error(...) (already done on line 101) and replace the $output->warning() message with a generic user-facing string such as 'Decidesk configuration import failed — see server log for details.'


Realistic-looking PII in seed data
Rule: OWASP A02:2021 – Cryptographic / Data Exposure (informational)
File: lib/Settings/decidesk_register.json (Participant seeds, lines ~317–350)
Issue: Seed data includes realistic Dutch names and @westerkwartier.nl / @waterschap-aaenmaas.nl email addresses (e.g. r.devries@westerkwartier.nl). These are development/demo fixtures, but if the repair step (InitializeSettings) runs on a production instance, this data will be imported into the OpenRegister data store. Westerkwartier and Waterschap Aa en Maas are real Dutch municipalities — the email addresses follow the pattern of real civil-servant addresses, creating an unnecessary data-residency risk.
Fix: Replace email seeds with clearly fictional addresses (e.g. r.devries@example.nl) to prevent any ambiguity about whether real PII is being stored.


Conduction ADR Compliance

Rule Status
Nextcloud auth only — no custom login/sessions ✅ Pass — no auth code introduced
Admin check via IGroupManager::isAdmin() on backend ✅ Pass — no admin authorization logic in changed files
No PII in logs/error responses ✅ Pass (see suggestion above for minor hardening)
Public endpoints annotated #[PublicPage] ✅ N/A — no new endpoints in this PR
No stack traces in API responses ✅ Pass — exceptions caught and logged, not exposed via API
mailEnabled: true on Decision schema ✅ Expected — documented in PR as intentional for _mail metadata

False Positives Suppressed

[FALSE POSITIVE] Postman collection admin/admin credentials (tests/integration/app-template.postman_collection.json:31-38) — These are Postman environment variable placeholders ({{admin_user}}, {{admin_password}}), not hardcoded secrets. The literal strings admin are default values for a local development environment, not production credentials.

@rubenvdlinde rubenvdlinde removed the ready-for-security-review Code review complete — awaiting security reviewer label Apr 13, 2026
@rubenvdlinde
Copy link
Copy Markdown
Contributor Author

Security Review — Clyde Barcode

Result: PASS (0 critical, 2 warning, 2 suggestion)

Scope

Changed files reviewed: PHP sources, Vue components, JS store, JSON schema/seed data, test files.
SAST: Semgrep p/security-audit + p/secrets + p/owasp-top-ten — 144 rules, 10 files, 0 findings.


WARNING

Secret ballot integrity gap — Vote schema is unconditionally searchable
Rule: OWASP A01:2021 (Broken Access Control)
File: lib/Settings/decidesk_register.json (Vote schema, x-openregister.searchable: true)
Issue: The Vote schema has searchable: true, meaning individual vote records are queryable via OpenRegister's search API. The VotingRound schema has an isSecret boolean, but that flag has no effect on the Vote schema's searchability. In a secret ballot, an actor with read access to the OpenRegister objects endpoint could query all Vote records for a given VotingRound and correlate participant UUIDs to their votes, violating the secret ballot guarantee.
Fix: Either (a) set searchable: false on the Vote schema and expose aggregated counts only through VotingRound, or (b) implement an access-control layer in the Vote controller that suppresses participant-linked fields when votingRound.isSecret = true. This must be resolved before the voting feature is exposed to end-users.

Decision mailEnabled without visible input sanitization controls
Rule: OWASP A03:2021 (Injection)
File: lib/Settings/decidesk_register.json (Decision schema, x-openregister.mailEnabled: true)
Issue: mailEnabled: true activates OpenRegister's email-to-object ingestion for the Decision schema. Inbound emails are external, untrusted input. This PR enables the feature but contains no email-parsing controller, sanitizer, or input validation logic. If the OpenRegister mail handler stores raw message content (headers, body, HTML) without sanitization, it creates a stored-XSS or injection vector in the decisions list.
Fix: Before enabling mailEnabled in production, ensure OpenRegister's mail ingestion pipeline strips or sanitizes HTML, validates sender identity, and that Decidesk's Decision views use Vue's default HTML-escaping (not v-html). Add a ticket to audit the mail ingest path prior to the first release.


SUGGESTION

Seed data references real Dutch government email domains
File: lib/Settings/decidesk_register.json (Participant seeds, lines 324–350)
Issue: Seed data includes email addresses at real municipal domains (@westerkwartier.nl, @waterschap-aaenmaas.nl). These are auto-imported on install via InitializeSettings. If personas are fictional, consider using a clearly-fake domain (e.g. @example.gemeente.nl) to avoid any GDPR ambiguity and prevent accidental phishing-profile creation.
Fix: Replace seed email domains with @example.gemeente.nl or similar clearly fictional domain.

Postman collection contains hardcoded default admin credentials
File: tests/integration/app-template.postman_collection.json:32-38
Issue: The collection defines default variable values admin_user: admin / admin_password: admin. While these are Postman collection variables (not secrets), developers who import this collection and run it against a non-local environment could inadvertently authenticate with production default credentials if those were never changed. Gitleaks is expected to suppress this (Postman variables, not a literal secret), but worth noting for hygiene.
Fix: Change the default to an empty string or a placeholder like <your-admin-user> to prevent accidental use against non-local targets.


False Positives Suppressed

[FALSE POSITIVE] generic-hardcoded-password — admin/admin in Postman collection are Postman variable default values for a local development environment, not real credentials embedded in application code.


Conduction Security Rules Check

Rule Status
Auth: Nextcloud built-in only (no custom login) ✅ No custom auth introduced
Admin checks via IGroupManager::isAdmin() on backend ✅ Not applicable to this PR (no new admin checks)
No PII in logs/error responses ✅ Exception messages logged at appropriate level, no stack traces in user-facing output
Public endpoints annotated #[PublicPage] ✅ No new public endpoints in this PR
CORS properly scoped ✅ Not applicable

…s to 1.15.0 (#4)

- eslint.config.js: add @SPEC to definedTags so OpenSpec traceability
  annotations no longer trigger jsdoc/check-tag-names warnings
- package-lock.json: npm audit fix upgrades axios from 1.13.6 → 1.15.0,
  resolving GHSA-3p68-rc4w-qgx5 and GHSA-fvcv-3m26-pcqx (critical SSRF)
  and @nextcloud/axios from 2.5.1 → 2.5.2

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@rubenvdlinde
Copy link
Copy Markdown
Contributor Author

Hydra Builder — Quality Fix

Fixed quality failures from CI run 24336061861:

Fixed

  • [ESLint] Invalid JSDoc tag name `spec` (src/store/store.js:9):
    Added @spec to the jsdoc/check-tag-names definedTags list in eslint.config.js. This tag is used across the codebase for OpenSpec traceability links and is intentional.

  • [npm audit] Critical SSRF in axios (GHSA-3p68-rc4w-qgx5, GHSA-fvcv-3m26-pcqx):
    Upgraded axios from 1.13.61.15.0 and @nextcloud/axios from 2.5.12.5.2 via npm audit fix. No breaking changes — both upgrades stay within the ^1.x semver range. The npm audit --audit-level=critical --omit=dev check now passes cleanly.

Remaining (non-critical, informational)

  • Vue 2 ReDoS (GHSA-5j4c-8p2g-v4jx): fix requires upgrading to Vue 3, which is a breaking change for the entire Nextcloud Vue 2 ecosystem. Not addressed — out of scope for this PR.

@github-actions
Copy link
Copy Markdown
Contributor

Quality Report

Repository ConductionNL/decidesk
Commit 5ede08f
Branch 5/merge
Event pull_request
Generated 2026-04-13 09:43 UTC
Workflow Run https://github.com/ConductionNL/decidesk/actions/runs/24336597585

Summary

Group Result
PHP Quality PASS
Vue Quality PASS
Security PASS
License FAIL
PHPUnit FAIL
Newman PASS
Playwright SKIP

PHP Quality

Tool Result
lint PASS
phpcs PASS
phpmd PASS
psalm PASS
phpstan PASS
phpmetrics PASS

Vue Quality

Tool Result
eslint PASS
stylelint PASS

Security

Ecosystem Result
composer PASS
npm PASS

License Compliance

Ecosystem Result
composer PASS
npm FAIL

composer dependencies (100 total)

Metric Count
Approved (allowlist) 100
Approved (override) 0
Denied 0

npm dependencies (410 total)

Metric Count
Approved (allowlist) 409
Approved (override) 0
Denied 1

Denied packages

Package Version License
apexcharts 5.10.6 Custom: https://apexcharts.com/media/apexcharts-logo.png

PHPUnit Tests

PHP Nextcloud Result
Overall FAIL

Code coverage: 0% (0 / 3 statements)

Integration Tests (Newman)

Result
PASS

E2E Tests (Playwright)

Playwright E2E tests were not enabled for this run.


Generated automatically by the Quality workflow.

Download the full PDF report from the workflow artifacts.

[WARNING] testRegistersAllSeventeenSchemas false positive:
- Renamed the misleading test to testIgnoresEventThatIsNotDeepLinkRegistrationEvent
- Added a DeepLinkRegistrationEvent stub (tests/Stubs/) loaded via bootstrap-unit.php
  so the real happy path can be exercised without OpenRegister as a Composer dep
- Added testRegistersAllSeventeenSchemas: asserts exactly 17 register() calls with
  correct appId, registerSlug, and URL template per schema slug
- Added testRegistersExpectedSlugsAndUrlTemplates: verifies each slug maps to the
  correct /apps/decidesk/#/{route}/{uuid} template
- Also registered OCP namespace in bootstrap-unit.php for standalone test runs
- Fixed pre-existing expects(constraint:) named-param errors in InitializeSettingsTest

[WARNING] Vote schema unconditionally searchable (secret ballot risk):
- Set Vote.x-openregister.searchable to false; individual vote records are no longer
  queryable via the OpenRegister search API, preventing participant-to-vote correlation
  in secret ballot VotingRounds

[WARNING] Decision mailEnabled without sanitization controls documented:
- Added x-mail-security-audit annotation to Decision schema referencing issue #6
- Created tracking issue #6 with pre-release audit checklist (HTML sanitization,
  sender validation, Vue v-html audit)

[SUGGESTION - security] Seed data PII — realistic Dutch municipal email domains:
- Replaced @westerkwartier.nl and @waterschap-aaenmaas.nl in Participant seeds
  with @example.gemeente.nl and @example.waterschap.nl (clearly fictional)
@rubenvdlinde
Copy link
Copy Markdown
Contributor Author

Hydra Builder — Fix iteration 1

Fixed findings:

  • [WARNING] testRegistersAllSeventeenSchemas does not test what its name claims (tests/Unit/Listener/DeepLinkRegistrationListenerTest.php:77):
    Renamed the misleading test to testIgnoresEventThatIsNotDeepLinkRegistrationEvent. Added a DeepLinkRegistrationEvent stub (tests/Stubs/DeepLinkRegistrationEvent.php) loaded in bootstrap-unit.php so tests run standalone without the OpenRegister app. Added two new tests: testRegistersAllSeventeenSchemas (asserts exactly 17 register() calls with correct appId, registerSlug, and URL template pattern) and testRegistersExpectedSlugsAndUrlTemplates (verifies each schema slug maps to the exact expected /apps/decidesk/#/{route}/{uuid} URL). Also registered OCP namespace in the bootstrap for standalone PHPUnit runs, and fixed pre-existing expects(constraint:) named-parameter errors in InitializeSettingsTest.

  • [WARNING] Vote schema unconditionally searchable — secret ballot integrity gap (lib/Settings/decidesk_register.json):
    Set Vote.x-openregister.searchable to false. Individual vote records are no longer queryable via OpenRegister's search API, preventing correlation of participant UUIDs to their votes in secret ballot VotingRounds.

  • [WARNING] Decision mailEnabled without visible input sanitization controls (lib/Settings/decidesk_register.json):
    Added x-mail-security-audit annotation to Decision.x-openregister referencing issue Security audit: Decision mailEnabled — verify mail ingest sanitization before production #6. Created issue Security audit: Decision mailEnabled — verify mail ingest sanitization before production #6 with a pre-release audit checklist covering HTML sanitization, sender identity validation, and Vue v-html audit for Decision views.

Additional fix (security SUGGESTION, both reviews):

  • [SUGGESTION] Realistic Dutch municipal email domains in Participant seeds (lib/Settings/decidesk_register.json):
    Replaced @westerkwartier.nl and @waterschap-aaenmaas.nl seed addresses with @example.gemeente.nl and @example.waterschap.nl to eliminate any GDPR ambiguity.

Remaining SUGGESTIONs (not addressed — informational only):

  • getName() references ConfigurationService instead of SettingsService (lib/Repair/InitializeSettings.php:59)
  • Stale Dutch summary in appinfo/info.xml:8
  • No seed data for 6 non-core schemas (DigitalDocument, MonetaryAmount, etc.)
  • Postman collection default admin credentials

Quality gate (25/25 tests, 420 assertions, 0 failures):

phpcs: 8/8 files — 0 errors ✓
phpunit: 25 tests, 420 assertions — 0 failures ✓

rubenvdlinde added a commit that referenced this pull request Apr 13, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Quality Report

Repository ConductionNL/decidesk
Commit 5e1c057
Branch 5/merge
Event pull_request
Generated 2026-04-13 09:55 UTC
Workflow Run https://github.com/ConductionNL/decidesk/actions/runs/24337095422

Summary

Group Result
PHP Quality PASS
Vue Quality PASS
Security PASS
License FAIL
PHPUnit FAIL
Newman PASS
Playwright SKIP

PHP Quality

Tool Result
lint PASS
phpcs PASS
phpmd PASS
psalm PASS
phpstan PASS
phpmetrics PASS

Vue Quality

Tool Result
eslint PASS
stylelint PASS

Security

Ecosystem Result
composer PASS
npm PASS

License Compliance

Ecosystem Result
composer PASS
npm FAIL

composer dependencies (100 total)

Metric Count
Approved (allowlist) 100
Approved (override) 0
Denied 0

npm dependencies (410 total)

Metric Count
Approved (allowlist) 409
Approved (override) 0
Denied 1

Denied packages

Package Version License
apexcharts 5.10.6 Custom: https://apexcharts.com/media/apexcharts-logo.png

PHPUnit Tests

PHP Nextcloud Result
Overall FAIL

Code coverage: 0% (0 / 3 statements)

Integration Tests (Newman)

Result
PASS

E2E Tests (Playwright)

Playwright E2E tests were not enabled for this run.


Generated automatically by the Quality workflow.

Download the full PDF report from the workflow artifacts.

- Override apexcharts to 4.7.0 (MIT) in package.json; npm audit fix had
  upgraded @conduction/nextcloud-vue from 0.1.0-beta.3 to 0.1.0-beta.8,
  which pulled in vue-apexcharts -> apexcharts 5.10.6 (proprietary license)
- Add DeepLinkRegistrationEvent stub loading to tests/bootstrap.php so
  PHPUnit integration runs (phpunit.xml) can find the class when OpenRegister
  is not installed in the Nextcloud test environment

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@rubenvdlinde
Copy link
Copy Markdown
Contributor Author

Hydra Builder — Fix iteration 2

Fixed CI failures introduced by the previous iteration's npm audit fix:

Fixed findings:

  • [LICENSE FAIL] apexcharts 5.10.6 denied license (package.json, package-lock.json):
    The npm audit fix in iteration 1 upgraded @conduction/nextcloud-vue from 0.1.0-beta.30.1.0-beta.8, which brought in vue-apexchartsapexcharts 5.10.6. ApexCharts v5 changed its license from MIT to a proprietary "SEE LICENSE IN LICENSE" (now rejected by the license checker). Fixed by adding "apexcharts": "4.7.0" to overrides in package.json, forcing the MIT-licensed 4.x build. npm audit --audit-level=critical passes cleanly.

  • [PHPUnit FAIL] Bootstrap missing stub for DeepLinkRegistrationEvent (tests/bootstrap.php):
    The CI runs the full PHPUnit suite via phpunit.xml (bootstrap tests/bootstrap.php) in a Nextcloud environment where OpenRegister is not installed. The DeepLinkRegistrationEvent class was not available, causing test errors. Fixed by adding the same stub-loading guard already present in tests/bootstrap-unit.php — the stub is loaded only when the real class is absent.

Quality gate:

phpcs: 8/8 files — 0 errors ✓
phpunit (unit): 25 tests, 420 assertions — 0 failures ✓
npm audit --audit-level=critical: 0 critical vulnerabilities ✓

Remaining SUGGESTIONs (not addressed — informational only):

  • getName() references ConfigurationService instead of SettingsService (lib/Repair/InitializeSettings.php:59)
  • Stale Dutch summary in appinfo/info.xml:8
  • No seed data for 6 non-core schemas
  • Postman collection default admin credentials

rubenvdlinde added a commit that referenced this pull request Apr 13, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Quality Report

Repository ConductionNL/decidesk
Commit f5b2373
Branch 5/merge
Event pull_request
Generated 2026-04-13 10:15 UTC
Workflow Run https://github.com/ConductionNL/decidesk/actions/runs/24337959132

Summary

Group Result
PHP Quality PASS
Vue Quality PASS
Security PASS
License PASS
PHPUnit FAIL
Newman PASS
Playwright SKIP

PHP Quality

Tool Result
lint PASS
phpcs PASS
phpmd PASS
psalm PASS
phpstan PASS
phpmetrics PASS

Vue Quality

Tool Result
eslint PASS
stylelint PASS

Security

Ecosystem Result
composer PASS
npm PASS

License Compliance

Ecosystem Result
composer PASS
npm PASS

composer dependencies (100 total)

Metric Count
Approved (allowlist) 100
Approved (override) 0
Denied 0

npm dependencies (416 total)

Metric Count
Approved (allowlist) 416
Approved (override) 0
Denied 0

PHPUnit Tests

PHP Nextcloud Result
Overall FAIL

Integration Tests (Newman)

Result
PASS

E2E Tests (Playwright)

Playwright E2E tests were not enabled for this run.


Generated automatically by the Quality workflow.

Download the full PDF report from the workflow artifacts.

…ispatcher\Event not found (#4)

The DeepLinkRegistrationEvent stub extends OCP\EventDispatcher\Event.
bootstrap.php was include-ing it before Nextcloud's lib/base.php was
loaded, so the OCP class was not yet available in the CI environment.

Moved stub loading to after the NC bootstrap block. Added the same
vendor/nextcloud/ocp PSR-4 fallback that bootstrap-unit.php already
uses, so standalone (no NC server) runs also resolve OCP correctly.
@rubenvdlinde
Copy link
Copy Markdown
Contributor Author

Hydra Builder — Fix iteration 3

Fixed finding:

  • [PHPUnit FAIL] Class "OCP\EventDispatcher\Event" not found in bootstrap (tests/bootstrap.php:23-25):
    The DeepLinkRegistrationEvent stub extends OCP\EventDispatcher\Event. In the CI Nextcloud environment, tests/bootstrap.php was include_once-ing the stub at line 23 — before lib/base.php was loaded on line 29. So OCP\EventDispatcher\Event was not yet available, crashing the entire test run.

    Fix: Moved the stub-loading block to after the Nextcloud bootstrap section. Also added the same vendor/nextcloud/ocp PSR-4 fallback registration already used by bootstrap-unit.php, so standalone runs (no NC server checkout) also resolve OCP\* correctly.

Remaining SUGGESTIONs (not addressed — informational only):

  • getName() references ConfigurationService instead of SettingsService (lib/Repair/InitializeSettings.php:59)
  • Stale Dutch summary in appinfo/info.xml:8
  • No seed data for 6 non-core schemas (DigitalDocument, MonetaryAmount, etc.)
  • Postman collection default admin credentials

rubenvdlinde added a commit that referenced this pull request Apr 13, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Quality Report

Repository ConductionNL/decidesk
Commit ca8c997
Branch 5/merge
Event pull_request
Generated 2026-04-13 10:20 UTC
Workflow Run https://github.com/ConductionNL/decidesk/actions/runs/24338156311

Summary

Group Result
PHP Quality PASS
Vue Quality PASS
Security PASS
License PASS
PHPUnit PASS
Newman PASS
Playwright SKIP

PHP Quality

Tool Result
lint PASS
phpcs PASS
phpmd PASS
psalm PASS
phpstan PASS
phpmetrics PASS

Vue Quality

Tool Result
eslint PASS
stylelint PASS

Security

Ecosystem Result
composer PASS
npm PASS

License Compliance

Ecosystem Result
composer PASS
npm PASS

composer dependencies (100 total)

Metric Count
Approved (allowlist) 100
Approved (override) 0
Denied 0

npm dependencies (416 total)

Metric Count
Approved (allowlist) 416
Approved (override) 0
Denied 0

PHPUnit Tests

PHP Nextcloud Result
Overall PASS

Code coverage: 0% (0 / 3 statements)

Integration Tests (Newman)

Result
PASS

E2E Tests (Playwright)

Playwright E2E tests were not enabled for this run.


Generated automatically by the Quality workflow.

Download the full PDF report from the workflow artifacts.

@rubenvdlinde rubenvdlinde added ready-for-code-review Build complete — awaiting code reviewer ready-for-security-review Code review complete — awaiting security reviewer labels Apr 13, 2026
@rubenvdlinde
Copy link
Copy Markdown
Contributor Author

Code Review — Juan Claude van Damme

Result: PASS (0 critical, 3 warning, 2 suggestion)

Overall this is a well-executed P1 sprint. The 17-schema register JSON is complete and internally consistent, enum values line up with seed data, the @self-envelope seed format is correct, relation declarations match the test expectations, and the InitializeSettings repair step is properly guarded with an OpenRegister availability check and idempotent via slug upsert. Test coverage is solid: 12 RegisterJsonTest methods, 4 listener tests, 4 repair-step tests, and the SettingsControllerTest round out the surface area touched in this PR.


WARNING

License tag in info.xml contradicts actual license
File: appinfo/info.xml:38
Issue: <licence>agpl</licence> is declared, but every PHP file header and package.json state EUPL-1.2. The Nextcloud app store reads this field for license display and compatibility checks — the mismatch creates legal ambiguity and could cause an app store rejection.
Fix: Align with the intended license. If the app is EUPL-1.2, change the tag to eupl; if it is AGPL, update all PHP file headers and package.json accordingly.
Note: this line predates this PR; the PR only added the <repair-steps> block. Flagged because appinfo/info.xml is in scope.


Dutch <summary> and <description> are still template boilerplate
File: appinfo/info.xml:8, appinfo/info.xml:23–35
Issue: The lang="nl" summary reads "Een sjabloon voor het maken van nieuwe Nextcloud-apps" and the description block is also copied verbatim from the Nextcloud app template, including a typographical error on line 29: "Beheerdersinstell ingen" (spurious space mid-word). These will appear verbatim on the Dutch-locale app store page.
Fix: Replace both blocks with Decidesk-specific Dutch content describing the platform's governance meeting/decision functionality.
Note: pre-existing; flagged because the file is in scope.


getName() doc string names wrong service class
File: lib/Repair/InitializeSettings.php:59
Issue: The method returns 'Initialize Decidesk register and schemas via ConfigurationService' but the class depends on and uses SettingsService. This misleads anyone reading Nextcloud's repair-step output or grepping for ConfigurationService.
Fix: Change to 'Initialize Decidesk register and schemas via SettingsService'.


SUGGESTION

store.js registration loop has no unit test
File: src/store/store.js:42–44
Issue: The new OBJECT_TYPES loop and registerObjectType calls are not covered by a test. The logic is straightforward, but a snapshot/spy test would catch future accidental removals of schema registrations.
Fix: Consider a Jest test that mocks objectStore.registerObjectType and asserts it is called exactly 17 times with the expected slug pairs.


testMotionSchema does not cover the full lifecycle enum by value
File: tests/Unit/RegisterJsonTest.php:213–215
Issue: The test only checks that submitted and withdrawn are present and that the count is 6. The four intermediate values (debating, voting, adopted, rejected) are not asserted by name, so a future typo in the enum would go undetected.
Fix: Assert the full ordered enum ['submitted', 'debating', 'voting', 'adopted', 'rejected', 'withdrawn'] with assertSame, as done for the Meeting lifecycle enum in testMeetingLifecycleEnum.

@rubenvdlinde rubenvdlinde removed the ready-for-code-review Build complete — awaiting code reviewer label Apr 13, 2026
@rubenvdlinde
Copy link
Copy Markdown
Contributor Author

Security Review — Clyde Barcode

Result: PASS (0 critical, 1 warning, 1 suggestion)


Scan Coverage

  • SAST: Semgrep p/security-audit, p/secrets, p/owasp-top-ten — 16 changed source files scanned, 0 findings
  • Manual OWASP review: Changed PHP, JS/Vue, and JSON files reviewed against ADR-005 and ADR-002

WARNING

mailEnabled: true on Decision schema — email-ingestion security unverified
Rule: OWASP A03:2021 (Injection), A07:2021 (Authentication Failures)
File: lib/Settings/decidesk_register.json:979
Issue: The Decision schema enables inbound email ingestion via OpenRegister. Attacker-controlled emails could supply malicious HTML, forge sender identity, or inject content that is later rendered unsanitized in frontend views. The schema itself includes an annotation acknowledging these risks and references issue #6, but no mitigations are confirmed in this PR.
Fix: Before enabling in any non-dev environment: (1) confirm OpenRegister sanitizes HTML body and headers on ingestion, (2) add sender-identity validation (SPF/DKIM check or allowlist), (3) ensure all Decision views that render email-sourced text content use Vue's safe data binding (not v-html). Track remediation status in #6.


SUGGESTION

Exception message forwarded to admin-visible repair output
File: lib/Repair/InitializeSettings.php:99
Issue: $e->getMessage() is passed directly to $output->warning() during the repair/migration step. In most failure modes this is harmless (admin-only context), but connection strings or internal paths present in exception messages would be surfaced in the upgrade log.
Fix: Log full detail to $this->logger->error() (already done on line 101) and replace the $output->warning() message with a generic string such as `'Could not auto-configure Decidesk — see server log for details.'"


False Positives Suppressed

[FALSE POSITIVE] tests/integration/app-template.postman_collection.json:35-38admin_user: "admin" / admin_password: "admin" are Postman variable defaults for a local test environment, clearly not production credentials. Postman uses {{admin_password}} substitution at runtime.


ADR Compliance

Rule Status
Nextcloud auth only (no custom login/session)
Admin check via IGroupManager::isAdmin() on backend ✅ (no admin-only endpoints in this PR)
No PII in responses or logs ✅ (seed emails use example.gemeente.nl domain)
Public endpoints annotated #[PublicPage] ✅ (no new public endpoints)
No stack traces in API responses
No secrets or hardcoded credentials in diff

@rubenvdlinde rubenvdlinde removed the ready-for-security-review Code review complete — awaiting security reviewer label Apr 13, 2026
@rubenvdlinde
Copy link
Copy Markdown
Contributor Author

Security Review — Clyde Barcode

Result: PASS (0 critical, 1 warning, 2 suggestions)

Scan coverage: Semgrep p/security-audit, p/secrets, p/owasp-top-ten — 141 rules across 14 PHP/JS files, 0 automated findings. Manual OWASP review of all 168 changed files.


WARNING

mailEnabled: true on Decision schema activates unaudited email ingestion surface
Rule: OWASP A03:2021 (Injection — Stored XSS via mail-sourced content)
File: lib/Settings/decidesk_register.json:979
Issue: Setting mailEnabled: true on the Decision schema tells OpenRegister to activate inbound email ingestion for Decision objects. Email bodies can contain attacker-controlled HTML. If any Vue component ever renders Decision text or mail-derived fields via v-html (rather than as text content), this becomes a stored XSS vector. The schema's own x-mail-security-audit annotation acknowledges this risk and references issue #6 for follow-up — but the feature ships enabled in this PR.
Fix: Before production deployment, confirm: (1) OpenRegister sanitizes inbound HTML/headers before storing; (2) all Decision views in the frontend render text fields as plain text (never v-html); (3) sender identity validation is in place. Tracking issue #6 should be resolved or the risk explicitly accepted before promoting to production.


SUGGESTION

Postman collection ships with weak default credentials
Rule: OWASP A07:2021 (Identification and Authentication Failures)
File: tests/integration/app-template.postman_collection.json:31,36
Issue: Default variable values admin_user: "admin" / admin_password: "admin" are hardcoded. These are clearly local-dev placeholders (targeting localhost:8080), but shipping them in the repo normalises weak defaults and risks accidental use in CI environments without override.
Fix: Replace default values with placeholder tokens (e.g. {{NEXTCLOUD_ADMIN_USER}}) and document in a README that these must be set via environment variables before running integration tests.

No Content-Security-Policy defined for app
Rule: OWASP A05:2021 (Security Misconfiguration)
File: appinfo/info.xml
Issue: No app-level CSP is declared. Nextcloud provides a default CSP, but with mailEnabled activating email ingestion on the Decision schema, an explicit restrictive CSP is a valuable defence-in-depth layer against any XSS in mail-sourced data.
Fix: Define a CSP for the app once the mail ingestion feature is fully specified. At minimum ensure script-src does not allow unsafe-eval or unsafe-inline for Decision-related views.


False Positives Suppressed

  • [FALSE POSITIVE] admin/admin in Postman collection — test fixture for local dev only, not a real credential leak
  • [FALSE POSITIVE] $e->getMessage() in InitializeSettings.php:99 — output goes to IOutput (migration CLI), not HTTP responses; no PII exposure in API responses
  • [FALSE POSITIVE] Email addresses in Participant seed data (@example.gemeente.nl) — fictional demo data, not real PII

@rubenvdlinde rubenvdlinde marked this pull request as ready for review April 13, 2026 12:17
@rubenvdlinde
Copy link
Copy Markdown
Contributor Author

Pipeline complete — code review and security review both passed.

Fix iterations: 3.

1 similar comment
@rubenvdlinde
Copy link
Copy Markdown
Contributor Author

Pipeline complete — code review and security review both passed.

Fix iterations: 3.

@rubenvdlinde rubenvdlinde merged commit 49eb046 into development Apr 13, 2026
46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant