This repository was archived by the owner on May 29, 2026. It is now read-only.
fix(security): wave-3 critical fixes#97
Merged
Merged
Conversation
…ics comment TC1 (false-positive doc fix): Remove misleading @AuthorizedAdminSetting claim from MetricsController docblock. The endpoint IS admin-only by NC default (SecurityMiddleware enforces isAdminUser() when @NoAdminRequired is absent); the comment wrongly cited an attribute that was not applied. TC2: Fix canonical brand icons (app.svg, app-dark.svg, app-store.svg). - app.svg: fill="currentColor" -> fill="#fff" (was rendering as black silhouette) - app-store.svg: replace <rect rx="40"> + NC blue #0082C9 with canonical point-up hexagon on Conduction cobalt #21468B, no overlapping strokes - app-dark.svg: strip fill="white" -> no fill (inherits currentColor per convention) TC3: Synchronise l10n/en.json and l10n/nl.json to a shared key-set union. Previously en had 12 keys, nl had 29 keys, only 4 overlapping. Both files now contain all 43 keys gathered from Vue sources + manifest + existing locale files, with proper English values and Dutch translations throughout. Fixes: /tmp/triage-nldesign-nctemplate.md TC1, TC2, TC3
rubenvdlinde
requested review from
Rem-Dam,
SudoThijn,
WilcoLouwerse,
bbrands02,
remko48 and
rjzondervan
as code owners
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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
Fixes wave-3 CRITICAL findings from
/tmp/triage-nldesign-nctemplate.md.TC1 (false-positive doc fix) — Fix misleading MetricsController comment
The
@NoCSRFRequireddocblock onMetricsController::index()incorrectly claimed the endpoint was protected by@AuthorizedAdminSetting(an attribute that was never applied). The endpoint IS admin-only by Nextcloud default —SecurityMiddlewareenforcesisAdminUser()when@NoAdminRequiredis absent. Updated the comment to accurately explain the real enforcement mechanism.TC2 — Fix canonical brand icons
All three brand icon files violated the fleet convention:
img/app.svg:fill="currentColor"→fill="#fff"(was rendering as black silhouette in NC nav)img/app-store.svg: replaced<rect rx="40">(rounded rectangle) on NC blue#0082C9with the canonical point-up hexagon on Conduction cobalt#21468B, removed overlappingstrokepathimg/app-dark.svg: removedfill="white"→ no fill attribute (correct: inherits currentColor)Every app scaffolded from this template inherited the broken icons fleet-wide.
TC3 — Synchronise l10n key sets
l10n/en.jsonhad 12 keys andl10n/nl.jsonhad 29 keys with only 4 in common, violating the apps-must-support-nl+en policy. Built the union of all 43 keys used across Vue sources, PHP lib, and manifest strings. Both locale files now contain the complete key set with correct English values and Dutch translations.Follow-up
A Hydra issue should be filed to add:
Test plan
app.svg: renders as white glyph in NC nav (not black silhouette)app-store.svg: displays point-up hexagon with cobalt backgroundapp-dark.svg: glyph inherits theme colour correctlyl10n/en.jsonandl10n/nl.jsonkey sets are identical🤖 Generated with Claude Code