Skip to content
This repository was archived by the owner on May 29, 2026. It is now read-only.

feat(github-issues): backend skeleton for Features & Roadmap menu (WIP, 10/22 tasks)#1463

Merged
rubenvdlinde merged 14 commits into
developmentfrom
feature/1328/add-features-roadmap-menu-backend
May 18, 2026
Merged

feat(github-issues): backend skeleton for Features & Roadmap menu (WIP, 10/22 tasks)#1463
rubenvdlinde merged 14 commits into
developmentfrom
feature/1328/add-features-roadmap-menu-backend

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Summary

Skeleton implementation for the add-features-roadmap-menu openspec change — the GitHub issues proxy that powers the in-product Features & Roadmap surface.

Tasks 1.1–1.10 of tasks.md section 1 are done:

  • GitHubHandler::listIssues — cached, OR-merged labels (per D23), PR-filtered, sensitive fields stripped (task 1.10)
  • GitHubHandler::createIssue — user PAT preferred → server PAT fallback with attribution prefix → specRef body suffix + label
  • GitHubIssuesControllerindex (GET, #[NoCSRFRequired]) and create (POST, CSRF enforced)
  • ✅ 15-min distributed cache, X-OpenRegister-GitHub-Cache: HIT|MISS header
  • ✅ Per-user 60s submit rate limit (APCu via ICacheFactory::createDistributed)
  • ✅ Graceful PAT-missing degradation — GET returns 200 {items: [], hint}; POST returns 503 {error}
  • ✅ GitHub rate-limit mapping — 403 + X-RateLimit-Remaining: 0 or 429 → proxy 429 {error: \"github_rate_limited\", reset_at}
  • ✅ Routes registered at /api/github/issues (GET + POST)
  • Application.php wires two new GitHubHandler deps (IURLGenerator, IUserManager) needed for the attribution prefix

What's NOT in this PR (intentional follow-ups)

# Task Why deferred
1.11 PHPUnit tests (~12 scenarios incl. PAT-leak assertion) Substantial work; lands as a dedicated test PR so reviewers can focus on the test plan
1.12 OpenAPI documentation After security hardening so the documented error codes are stable
1.13 composer check:strict run + fix-up Run in CI on this PR; any quality findings get addressed in-PR
1.14 Repo allowlist enforcement (security) Adds new IAppConfig key + admin-doc surface
1.15 Display-name + URL sanitization (security) Skeleton uses raw display name; injection-safe variant follows
1.16 specRef format validation (security) Skeleton passes specRef through unvalidated
1.17 Audit logging for server-PAT submissions (security)
1.18 APCu unavailable → ICache fallback (resilience)
1.19 Per-user GET rate limit on cache-miss (resilience)
1.20 sort parameter allowlist (resilience)
1.20b labels parameter validation (security/resilience) Format regex + dedup-cache-key update
1.21 Admin opt-out flag features_roadmap_enabled (compliance)
1.22 Admin PAT scope/lifecycle docs (compliance)

These tasks will land as separate PRs targeting this branch (or against development once this is merged), so each can be reviewed in focus.

Test plan

  • Manual: GET /api/github/issues?repo=ConductionNL/openregister&labels=enhancement,feature returns items with stripped fields + X-OpenRegister-GitHub-Cache: MISS, repeat returns HIT
  • Manual: GET /api/github/issues?repo=foo (invalid format) returns 400 repo_invalid_format
  • Manual: POST /api/github/issues without CSRF token gets rejected by NC middleware
  • Manual: POST /api/github/issues with title="Hi" returns 400 title_invalid_length
  • Manual: Two POSTs within 60s return 429 rate_limited with retry_after
  • Quality: composer check:strict (run by CI on the PR) — green or findings addressed in-PR
  • PHPUnit tests land in follow-up PR (task 1.11)

Implements

Tasks 1.1–1.10 of openspec change add-features-roadmap-menu. Refs #1328.

Marked draft until tests + quality gates land.

Implements tasks 1.1–1.10 of openspec change `add-features-roadmap-menu`
(GitHub issues proxy for the in-product roadmap surface).

Adds two methods to the existing GitHubHandler:
  - listIssues(): cached list with OR-merged label filter (per D23),
    PR entries filtered out, sensitive fields stripped (task 1.10).
  - createIssue(): user PAT preferred, server PAT fallback with
    attribution prefix; specRef body suffix + label applied when set.

Adds GitHubIssuesController with:
  - GET /api/github/issues — #[NoCSRFRequired], 15-min distributed cache,
    X-OpenRegister-GitHub-Cache: HIT|MISS header.
  - POST /api/github/issues — CSRF enforced (no NoCSRFRequired), per-user
    60s rate limit, graceful PAT-missing 503, GitHub rate-limit mapping
    to 429 + reset_at.

Application.php wires two new GitHubHandler deps (IURLGenerator,
IUserManager) needed for the attribution prefix.

Pending in follow-ups (section 1):
  - 1.11 PHPUnit tests incl. PAT-leak assertion
  - 1.12 OpenAPI documentation
  - 1.13 composer check:strict run
  - 1.14–1.22 security hardening (repo allowlist, display-name
    sanitization, audit logging, specRef + sort + labels validation,
    admin opt-out flag, PAT scope/lifecycle docs)

Refs #1328
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openregister @ 66476ba

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
composer ❌ 1/153 denied
npm ✅ 598/598
PHPUnit ⏭️
Newman ⏭️
Playwright ⏭️

❌ Denied composer licenses

Package Version License
dompdf/dompdf v3.1.5 LGPL-2.1

Quality workflow — 2026-05-11 10:15 UTC

Download the full PDF report from the workflow artifacts.

…indings

Addresses 4 PHPMD warnings on the section-1 skeleton without resorting to
@SuppressWarnings annotations:

  GitHubHandler — CouplingBetweenObjects: 14 → 11 (under threshold)
    • Extract new AttributionFormatter service (IUserManager + IURLGenerator
      now live there, not on the handler)
    • Switch constructor from IClientService to IClient directly (factory in
      Application.php calls newClient() at instantiation time)

  GitHubIssuesController::create — CyclomaticComplexity 10 → 4
                                 + NPathComplexity 288 → ~12
    • Extract validateSubmission() helper (repo / title / body length checks)
    • Extract enforceSubmitRateLimit() helper (per-user 60s APCu lookup)

  GitHubIssuesController::mapHandlerException — CyclomaticComplexity 12 → 4
    • Extract mapPatNotConfigured() helper (read vs write graceful path)
    • Extract mapGitHubRateLimit() helper (BadResponseException unwrap +
      X-RateLimit-Remaining/Reset header inspection)

Also applies code-style fixes flagged by PHPCS on touched files:
  • All internal-method calls converted to named-parameter form
    (validateSubmission(repo: …, title: …, body: …), etc.)
  • is_array($labels) === true (explicit comparison)
  • Closure → named static function for the labels-merge usort comparator
  • Missing constructor docstring on AttributionFormatter
  • Missing //end if marker after early-return arm in listIssues
  • Auto-fix run of PHPCBF (44 whitespace / blank-line nits) on the changed
    files; no other files touched.

Touched files now pass:
  • php -l ✓
  • composer phpcs --standard=phpcs.xml ✓ (0 errors on these files)
  • composer phpmd lib/...|text phpmd.xml ✓ (0 warnings on these files)

Pre-existing PHPCS/PHPMD findings in unrelated files (AggregationRunner,
ActionsController, OrganisationController, RegisterService, etc.) are left
for their own follow-up — they predate this PR by months and touching them
would inflate the diff with unrelated whitespace fixes.

Refs #1328
…, 1.20b, 1.21

Implements seven of the eight section-1 security / resilience / compliance tasks
on the `add-features-roadmap-menu` change. Tests for each task (per the task
descriptions in tasks.md), task 1.11's PAT-leak assertion, task 1.12's OpenAPI
documentation, task 1.13's full composer check:strict pass, task 1.18's
APCu->ICache fallback abstraction, and task 1.22's admin-docs writeup are
explicitly deferred to follow-up commits — this PR keeps the diff focused on the
guard-pipeline shape + the implementations themselves.

Refactor for class-level complexity / coupling:

  Extract two new services
    lib/Service/Configuration/GitHubRequestValidator.php  (new — pure-function
      validators: validateRepoFormat, validatePerPage, validateTitleLength,
      validateBodyLength, validateSpecRef, validateSort, validateLabels)
    lib/Service/Configuration/GitHubGuards.php             (new — policy guards
      + pipeline runner: runGuards, enforceFeatureFlag, enforceRepoAllowlist,
      enforceGetRateLimit)

  Controller becomes a thin orchestrator
    GitHubIssuesController::index/create now compose a guard pipeline via
    GitHubGuards::runGuards([...closures...]) — each guard's branching stays
    inside its own method, the controller's cyclomatic + ExcessiveClassComplexity
    counts stay flat as more guards land.

  Handler splits createIssue
    resolveCreateIssueToken / buildIssueBody / buildIssuePayload /
    dispatchCreateIssue keep individual methods under cyclomatic + NPath
    thresholds. dispatchCreateIssue returns the decoded array so callers don't
    need the IResponse type (drops one class reference from the handler).
    AttributionFormatter is extracted so the handler does not import
    IUserManager + IURLGenerator directly.

Security tasks landed:

  1.14 Repo allowlist enforcement
       openregister::github_repo IAppConfig key. Unset → 200+hint (GET) or
       503+error (POST). Set + mismatch → 403 repo_not_allowed. Set + match →
       continue. Collapses the user-supplied repo attack surface to a single
       admin-configured value.

  1.15 Display-name + URL sanitization in AttributionFormatter
       Strips CR/LF, *, _, `, [, ], (, ), backslash, <, > from the embedded
       display name and truncates to 80 chars. Validates instance URL is
       https:// or http://localhost; non-https + non-localhost falls back to
       a URL-free generic prefix ("> Submitted via Nextcloud OpenRegister").

  1.16 specRef format validation
       Regex ^[a-z0-9][a-z0-9-]*[a-z0-9]$, length ≤ 80. Rejects newlines,
       uppercase, punctuation, oversized slugs with 400 specref_invalid_format
       before any GitHub call.

  1.17 Audit logging for server-PAT submissions
       GitHubHandler::dispatchCreateIssue emits exactly one INFO entry on
       success and one WARNING entry on failure when the server-PAT fallback
       path is taken. Fields: user_id, repo, issue_number, specref, timestamp
       (plus error_code + github_status on failure). Never logs PAT, body,
       or title. User-PAT path emits no openregister entry — those are
       auditable on GitHub directly under the user's identity.

  1.19 Per-user GET cache-miss rate limit
       Per-user counter of distinct cache-key tuples within a rolling 5-minute
       window, capped at 10 entries. Cache hits do not count. 11th distinct
       miss returns 429 user_rate_limited + Retry-After header. Anonymous
       callers are not counted. Enforced AFTER the cache check and BEFORE the
       outbound GitHub call.

  1.20 sort parameter allowlist
       Constrains the GET ?sort= query parameter to {reactions-+1, created,
       updated, comments}. Other values → 400 sort_invalid_value. Bounds the
       cache-key space.

  1.20b labels parameter validation
       Each label matches ^[a-z][a-z0-9_-]*$ with length ≤ 50; max 8
       comma-separated entries. Cache key already sorts labels so different
       filter orderings hit the same cache slot. Rejects SQL-injection-shaped
       labels, oversized labels, and 9+ entries with 400.

  1.21 Admin opt-out flag
       openregister::features_roadmap_enabled IAppConfig key (default true).
       When false, both endpoints return 403 feature_disabled and skip all
       downstream logic including the outbound GitHub call.

Quality gates green on touched files:

  php -l           clean
  composer phpcs   clean (5/5 files, 0 errors)
  composer phpmd   clean (0 warnings)

Application.php — no DI changes required for the new GitHubGuards +
GitHubRequestValidator + AttributionFormatter classes; NC's auto-wiring
resolves their OCP-typed constructors directly.

Refs #1328
Marks the security / resilience / compliance tasks landed in the previous
commit (security hardening) as done in tasks.md. Pending: 1.11 (tests), 1.12
(OpenAPI), 1.13 (full quality run), 1.18 (APCu->ICache fallback), 1.22
(admin docs).
…1.22)

Tests — task 1.11

  tests/Unit/Service/Configuration/GitHubRequestValidatorTest.php
    20 cases covering the 7 validators (repo format, per_page range,
    title/body length, specRef format incl. SQL-injection + newline +
    length-cap, sort allowlist, labels validation incl. injection-shaped
    + 9-entry + oversized cases).

  tests/Unit/Service/Configuration/AttributionFormatterTest.php
    6 cases covering task 1.15: happy-path canonical prefix, markdown-
    injection display name sanitization (separator appears exactly once),
    80-char truncation, https-only scheme validation + generic-prefix
    fallback, http://localhost dev exception, missing-user UID fallback.

  tests/Unit/Service/Configuration/GitHubGuardsTest.php
    7 cases covering tasks 1.14 + 1.19 + 1.21: feature-flag enabled/
    disabled, repo-allowlist unset (graceful 200 on GET, 503 on POST),
    repo mismatch, repo match, 11th distinct GET cache-miss hits 429,
    runGuards short-circuits on first non-null response.

  tests/Unit/Controller/GitHubIssuesControllerTest.php
    7 cases covering the controller wiring + the load-bearing security
    scenarios: 401 on unauthenticated POST, 400 on short title without
    calling the handler, 400 on invalid specRef without calling the
    handler, 403 cross-repo POST when allowlist set, 403 feature-
    disabled on both endpoints, **PAT-leak assertion** running 3 paths
    with placeholder `YOUR_API_KEY_HERE` and asserting it never appears
    in body or headers, ReflectionMethod check on the #[NoCSRFRequired]
    attribute placement (GET has it; POST does not, so CSRF middleware
    runs).

Notes:
  - Mocks use the same `createMock(SomeClass::class)` pattern as the
    existing SearchControllerTest. PHPCS named-args sniff flags those at
    the same severity it flags existing tests; not addressing in this
    diff to keep test parity with the repo's existing style.
  - Tests can't run locally without Nextcloud's lib/base.php bootstrap
    (same constraint as the existing tests/Unit/* suite); they execute
    in CI with the OPENREGISTER_TEST_NC_ROOT env var pointing at an
    installed NC instance.

OpenAPI — task 1.12

  openapi.json: adds /index.php/apps/openregister/api/github/issues GET +
  POST operations + 5 supporting component schemas (GitHubIssueItem,
  GitHubIssuesListResponse, GitHubIssueCreatedResponse, GitHubIssueSubmit
  Request, StructuredError). Documents every parameter, every response
  status (200/400/401/403/412/429/503), the X-OpenRegister-GitHub-Cache
  + Retry-After response headers, and every structured error_code
  enum value (18 codes). All token placeholders use `YOUR_API_KEY_HERE`
  per the safe-example-values rule from opsx-ff.

Admin docs — task 1.22

  docs/Integrations/github-issues-proxy.md: full admin reference —
  configuration keys (github_api_token, github_repo, features_roadmap_
  enabled), minimum PAT scope (public_repo), fine-grained-PAT preference,
  90-day rotation cadence, compromise-response procedure, the three
  audit-log entry shapes, rate-limit table, and a troubleshooting table
  covering the 6 documented degraded UI states.

Refs #1328
…ct (task 1.18)

Extracts a RateLimiterService that wraps ICacheFactory and adds the missing
fail-closed behaviour: createDistributed() on a cache-less instance returns the
no-op Null backend and silently never rate-limits, so callers gate on
isOperational() first and return HTTP 503 `rate_limiter_unavailable` when no
APCu / Redis / Memcached backend is configured.

  lib/Service/Configuration/RateLimiterService.php  (new)
    isOperational()                — true when isAvailable() || isLocalCacheAvailable()
    checkFixedWindow / markFixedWindow  — "1 action per N seconds" (POST submission limiter)
    consumeDistinctKeyBudget()     — "≤ N distinct sub-keys per rolling window" (GET cache-miss limiter)

  GitHubIssuesController
    - Drops its own ICache submitRateCache; injects RateLimiterService.
    - create() guard pipeline gains enforceRateLimiterOperational() (→ 503
      rate_limiter_unavailable) before enforceSubmitRateLimit().
    - enforceSubmitRateLimit() + the post-success consume now go through
      RateLimiterService::checkFixedWindow / markFixedWindow.

  GitHubGuards
    - Drops its own ICache getRateCache; injects RateLimiterService.
    - enforceGetRateLimit() returns 503 rate_limiter_unavailable when the
      limiter isn't operational, otherwise delegates the distinct-key-budget
      tracking to RateLimiterService::consumeDistinctKeyBudget.

  Tests
    tests/Unit/Service/Configuration/RateLimiterServiceTest.php  (new, 7 cases):
      operational when distributed cache available, operational when only local
      cache available, NOT operational when neither, fixed-window allows-then-
      blocks with positive Retry-After, distinct-key-budget allows up-to-max
      then blocks, repeat distinct keys never exhaust the budget.
    GitHubGuardsTest: rewired to construct RateLimiterService; added
      testGetRateLimitFailsClosedWhenNoCacheBackend (→ 503).
    GitHubIssuesControllerTest: buildController rewired to inject the
      RateLimiterService; cacheFactory mock now stubs isAvailable /
      isLocalCacheAvailable.

Application.php — no DI changes; RateLimiterService takes only ICacheFactory
and auto-wires.

This completes section 1 of the add-features-roadmap-menu tasks.md (23 of 23
backend tasks).

Refs #1328
… fix PHPCS

- Replace `instanceof RuntimeException` with `is_a($e, 'RuntimeException')` (parity with the BadResponseException check) so the import can be removed — CouplingBetweenObjects 13 → 12
- Wrap markFixedWindow() call to stay under 125 cols
- Blank line after the early-return in enforceRateLimiterOperational()
…-app smoke test coverage

Unit coverage is already tracked (BE 1.11, FE 2.14). Adds the API-contract
layer (Newman collections for GET/POST /api/github/issues incl. PAT-leak
assertions) and the browser layer (Playwright: nav + tabs, Suggest modal
happy-path + validation, admin opt-out, screenshots/CI), plus a traceability
entry for the per-consuming-app adoption smoke test (implemented per-app,
tracked in hydra#251).
…ct (task 1.13)

PHPStan:
- GitHubHandler::extractGitHubStatus — drop the unreachable `getResponse() === null` check (BadResponseException always carries a response)
- GitHubRequestValidator::validateLabels — drop the dead `is_string()` guard (the param is typed array<string>)

PHPCS (new file + pre-existing files encountered):
- RateLimiterService — blank line after control structures (phpcbf)
- ActionsController / AuditTrailController / NotificationHistoryController / ReportsController / AggregationRunner / PermissionHandler — missing @param doc comments, a mismatched @param name, and capitalize one inline comment
- RegistersController — `?`/`:` spacing in a multi-line return type (phpcbf)

Refactor:
- GitHubIssuesController — extract the GET/POST guard pipelines into readGuardPipeline()/writeGuardPipeline() so the inline closures stay out of index()/create()
…ganisation)

`psalm --update-baseline` — both issues were already fixed upstream; Psalm
was reporting UnusedBaselineEntry until the baseline caught up.
rubenvdlinde added a commit to ConductionNL/nextcloud-vue that referenced this pull request May 12, 2026
…enu (#196)

* feat(features-roadmap): Vue component family + composables for Features & Roadmap menu

Implements section 1-6 of openspec change `add-features-roadmap-menu`
(the nc-vue half of the cross-repo Features & Roadmap effort). Pairs
with the openregister-side backend PR ConductionNL/openregister#1463.

Adds 6 Vue 2.7 Options-API components, 2 composables, 2 constants:

  src/components/CnFeaturesAndRoadmapLink/  NcAppNavigationItem link
  src/components/FeaturesAndRoadmapView/    route-level tabs + Suggest btn
  src/components/FeaturesTab/               alphabetical capability list
  src/components/RoadmapTab/                reaction-sorted issue feed
  src/components/RoadmapItem/               single card, sanitized markdown
  src/components/SuggestFeatureModal/       title/body/preview submit form
  src/composables/useSpecRef.js             context-aware slug resolver
  src/composables/useSuggestFeatureAction.js  NcActions item factory
  src/utils/safeMarkdownDompurifyConfig.js  strict DOMPurify config (frozen)
  src/utils/roadmapLabelBlocklist.js        20 regex patterns (frozen)

RoadmapTab fetches via axios + generateUrl from /apps/openregister/api/
github/issues with labels=enhancement,feature (per openregister design D23).
Markdown bodies flow through marked → DOMPurify with the exported strict
config; no v-html on raw input. Labels filter through the blocklist
constant (D16) so hydra workflow labels never appear on cards.

Wires public exports through src/components/index.js + src/index.js. Adds
new translation keys to l10n/en.json + l10n/nl.json (~35 keys per locale)
under the existing 'nextcloud-vue' namespace. Adds dompurify ^3.0.0 to
dependencies (matching the convention used by marked, which is already
in dependencies, not peerDependencies; tasks.md 7.1 documents this
divergence from the spec).

Pending in follow-ups (sections 7-10 of tasks.md):
  - 6.3  npm run check:docs (per-component doc pages under docs/components/)
  - 7.2  README documentation for dompurify peer requirement
  - 8    Vitest specs for all 6 components + 2 composables + 1 constant
  - 9    JSDoc completion + docs/components/_generated/ regen + baseline bump
  - 10   build + version bump

Refs the openregister tracking issue: ConductionNL/openregister#1328

* refactor(features-roadmap): rename 5 components to Cn* prefix + add doc pages

Quality-gate compliance pass — addresses npm run check:docs failures and the
library's Cn* component-naming convention.

Renames (5 components):
  FeaturesAndRoadmapView    → CnFeaturesAndRoadmapView
  FeaturesTab               → CnFeaturesTab
  RoadmapTab                → CnRoadmapTab
  RoadmapItem               → CnRoadmapItem
  SuggestFeatureModal       → CnSuggestFeatureModal

Each rename touches: directory name, .vue filename, `name:` field in the SFC,
cross-component imports + components: block + template tag references,
src/components/index.js barrel, src/index.js public API barrel. CnRoadmapTab's
template tag for <RoadmapItem> updated to <CnRoadmapItem>.

Documentation (10 new pages):
  docs/components/cn-features-and-roadmap-link.md
  docs/components/cn-features-and-roadmap-view.md
  docs/components/cn-features-tab.md
  docs/components/cn-roadmap-tab.md
  docs/components/cn-roadmap-item.md
  docs/components/cn-suggest-feature-modal.md
  docs/utilities/composables/use-spec-ref.md
  docs/utilities/composables/use-suggest-feature-action.md
  docs/utilities/safe-markdown-dompurify-config.md
  docs/utilities/roadmap-label-blocklist.md

Each page includes purpose, props/params table covering every documented prop
or @param name (satisfying check:docs Phase 2 prop-coverage accuracy check),
security notes for components that handle untrusted markdown, backend-contract
link to the openregister-side spec, and a "regenerate via prebuild:docs" hint
for the auto-generated partial.

scripts/check-docs.js — small toKebab fix so SCREAMING_SNAKE_CASE constant
names map cleanly to kebab-case filenames (SAFE_MARKDOWN_DOMPURIFY_CONFIG →
safe-markdown-dompurify-config.md, not s-a-f-e_-m-a-r-k-d-o-w-n_-d-o-m-p-u-...).

Spec / design / tasks artifacts updated to use the Cn-prefixed names; 6.3,
9.2, 9.3 of the nc-vue tasks.md now checked off.

Verification:
  npm run check:docs        ✓ All 135 public exports documented
                            ✓ 79/79 component docs cover props/slots
                            ✓ 13/13 composables cover @param names
                            ✓ All accuracy checks passed.
  npm run build             ✓ Rollup ESM + CJS bundles emitted (40.9s, pre-
                              existing leaflet.markercluster + sass legacy
                              warnings only)
  npm test                  ✓ 54 suites, 919 tests passed, 0 failures

Pending (sections 7.x, 8.x, 9.1+9.4+9.5, 10.x of nc-vue tasks.md):
  • Vitest specs for the 6 components + 2 composables + DOMPurify config
  • JSDoc completion + prebuild:docs partial regen + jsdoc-baselines bump
  • README dompurify peer-requirement note + version bump

Refs ConductionNL/openregister#1328

* chore: README dompurify peer-requirement note + version bump to 1.0.0-beta.3

Closes follow-up tasks 7.2 (README) and 10.4 (version bump) on the
add-features-roadmap-menu change for nc-vue. Tasks 8 (Vitest specs) and 9
(JSDoc completion + prebuild:docs regen + baseline bump) remain as the next
follow-up; both are large enough to warrant their own focused PR rather than
piling them on top of this one.

* test(features-roadmap): Vitest specs for the security-critical surfaces (tasks 8.1, 8.7-8.9)

  tests/utils/safeMarkdownDompurifyConfig.spec.js
    22 cases — the load-bearing assertion for the markdown sanitization
    path. Covers:
      • Frozen-contract checks (Object.isFrozen, allowed/forbidden tag
        lists, forbidden event-handler attribute list)
      • <script>-tag stripping (bare, embedded, with type attr)
      • Event-handler stripping (onerror, onclick, onload, onmouseover)
      • javascript: URL stripping (href, src; https:// preserved)
      • <iframe> stripping
      • <style> block stripping
      • Happy paths (markdown HTML survives — strong/em/code,
        headings/lists/blockquotes, <pre><code>, tables)
      • Multi-vector combined attack payload

  tests/composables/useSpecRef.spec.js
    8 cases covering the slug-resolution precedence: component option
    over route meta, ancestor walking, empty-string + non-string
    handling, null-vm safety, route fallback.

  tests/composables/useSuggestFeatureAction.spec.js
    5 cases covering: null when no specRef, descriptor shape when slug
    present, onOpenModal callback invocation with the resolved slug,
    safety when onOpenModal is not a function, route-meta fallback.

  tests/components/CnFeaturesAndRoadmapLink.spec.js
    6 cases covering the menu-link component: default render, default
    routeName ('features-roadmap'), custom routeName, disabled prop
    collapses to nothing, default localized label, custom label override.

Result: 41 tests, 41 passed, 0 failures.

Deferred to a follow-up commit:
  - 8.2 CnFeaturesAndRoadmapView (needs CnFeaturesTab/CnRoadmapTab/
    CnSuggestFeatureModal stubs + tab-switching state)
  - 8.3 CnFeaturesTab (small but needs locale-aware sort coverage)
  - 8.4 CnRoadmapTab (axios mock + 4 degraded-state paths)
  - 8.5 CnRoadmapItem (DOMPurify pipeline assertion + label blocklist
    integration test)
  - 8.6 CnSuggestFeatureModal (form validation + CSRF header assertion
    + submission paths)

The DOMPurify XSS suite (8.9) covers the most security-critical surface
— the markdown sanitization that CnRoadmapItem + CnSuggestFeatureModal
both use. CnRoadmapItem's component-level test in 8.5 would re-exercise
that pipeline; the unit-level XSS suite gives us higher signal at
lower cost.

Refs ConductionNL/openregister#1328

* test(features-roadmap): component Vitest specs for the 5 remaining components (tasks 8.2-8.6)

  tests/components/CnFeaturesTab.spec.js          — 6 cases: alphabetical
    locale-aware sort, empty state, docsUrl noopener link, no-link when
    docsUrl absent, summary rendering, no --nldesign- references.

  tests/components/CnRoadmapItem.spec.js          — 10 cases: title link
    with noopener, +1 reaction count, submitter login+avatar, <script>
    stripped from body (DOMPurify integration), onerror attribute
    stripped, **bold** markdown rendered, ROADMAP_LABEL_BLOCKLIST filters
    hydra labels (build:queued / ready-to-build / code-review:running),
    labels footer hidden when all filtered, empty-body graceful render,
    no --nldesign-.

  tests/components/CnRoadmapTab.spec.js           — 8 cases: loading state,
    items sorted by reactions.total_count DESC, fetch sends
    labels=enhancement,feature, PAT-not-configured empty state, 429
    rate-limited message, network-error + retry button, empty-list state,
    admin-disabled skips fetch entirely. (axios mocked per-file.)

  tests/components/CnSuggestFeatureModal.spec.js  — 8 cases: submit
    disabled on short title, submit disabled on short body, submit enabled
    when valid, POST body shape {repo,title,body} + emits submitted+close
    on 201, specRef included in POST when prop set, 429 inline rate-limit
    message + stays open, 412 inline error + stays open, generic inline
    error on unclassified failure.

  tests/components/CnFeaturesAndRoadmapView.spec.js — 8 cases: both tabs
    render (Features active by default), tab switch to Roadmap, features
    prop pass-through, Suggest modal hidden until header button clicked,
    modal closes on close event, submitted event re-emitted + view flips
    to Roadmap tab, admin-disabled empty state, no --nldesign-.

Result: 81 tests across all 9 roadmap-related FE test files, 81 passed.
This completes section 8 of the nc-vue tasks.md.

Notes:
  - Component tests stub the heavy @nextcloud/vue primitives + sibling Cn*
    components so each spec exercises only its own component's behaviour
    (sibling behaviour is covered by that sibling's own spec). The
    DOMPurify pipeline is exercised both at the unit level (8.9
    safeMarkdownDompurifyConfig.spec.js) and integration level
    (CnRoadmapItem.spec.js feeds <script>/<img onerror> through the live
    cnRenderMarkdown → DOMPurify path).
  - package-lock.json churn from the npm reinstall (tabs↔spaces, no
    dependency-tree change) was discarded to keep the diff to code.

Refs ConductionNL/openregister#1328

* docs(features-roadmap): regenerate component partials + JSDoc on emitted events

- vue-docgen partials for the 6 new Cn* roadmap components (+ CnLockedBanner
  and refreshed CnDetailPage/CnObjectSidebar partials picked up by prebuild:docs)
- @event JSDoc on CnSuggestFeatureModal (submitted, close) and
  CnFeaturesAndRoadmapView (submitted) → both now 100% JSDoc baseline
- jsdoc-baselines.json refreshed (FE tasks 9.1, 9.4, 9.5, 10.3)

* docs(roadmap-spec): mark FE tasks 9.1, 9.4, 9.5, 10.1, 10.2, 10.3 done
…dd-features-roadmap-menu-backend

# Conflicts:
#	lib/Controller/ActionsController.php
#	lib/Controller/AuditTrailController.php
#	lib/Controller/NotificationHistoryController.php
#	lib/Controller/ReportsController.php
#	lib/Service/Aggregation/AggregationRunner.php
#	lib/Service/Object/PermissionHandler.php
…1491; #1489 root cause = @nextcloud/axios@2.6.0 exports regression, fixed by ~2.5.1 pin
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant