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

feat(resources): admin-only resource upload endpoint#46

Merged
rubenvdlinde merged 1 commit into
developmentfrom
feature/impl-resource-uploads
Apr 30, 2026
Merged

feat(resources): admin-only resource upload endpoint#46
rubenvdlinde merged 1 commit into
developmentfrom
feature/impl-resource-uploads

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Summary

Implements the resource-uploads capability — a small admin-only mini file API for the binary branding assets (icons, widget images) MyDash widgets need to host directly, separate from the user's Files folder.

This PR delivers REQ-RES-001 through REQ-RES-005 from the spec proposal in PR #42 (feature/replica-spec-proposals).

What's in scope

  • REQ-RES-001POST /api/resources accepting raw JSON {base64: 'data:image/<type>;base64,...'}. Admin-only via IGroupManager::isAdmin (HTTP 403 otherwise). Multipart bodies are rejected with HTTP 415.
  • REQ-RES-002 — Allowed declared types: jpeg, jpg, png, gif, svg, webp (case-insensitive). Disallowed types → HTTP 400 / invalid_image_format. Missing/unparseable data URL → HTTP 400 / invalid_data_url. image/svg+xml is normalised to svg.
  • REQ-RES-003 — 5 MB hard cap on decoded bytes, enforced before invoking getimagesizefromstring (so the server cannot be tricked into loading an oversize blob into the image library). Raster MIME cross-check via getimagesizefromstring. Corrupt raster → corrupt_image. Mismatch → mime_mismatch. SVG validation is delegated to the sibling svg-sanitisation capability.
  • REQ-RES-004 — Storage via IAppData::getFolder('resources') (auto-created on first use). Filenames resource_<uniqid>.<ext> using uniqid('resource_', true) for high entropy. Never writes to the user's Files folder.
  • REQ-RES-005 — Standardised envelope. Success: {status: 'success', url, name, size}. Error: {status: 'error', error: <stable_code>, message} with the documented enum (forbidden, unsupported_media_type, invalid_data_url, invalid_image_format, file_too_large, mime_mismatch, corrupt_image, storage_failure). Raw exception messages NEVER leak to clients (defence-in-depth Throwable catch wraps unexpected paths into storage_failure).

What's out of scope (sibling PRs)

  • REQ-RES-006..009 (resource-serving change)GET /apps/mydash/resource/<filename> endpoint, ETag/Last-Modified handling, MIME header on serve, 404 on unknown filename. Will land separately.
  • REQ-RES-010..013 (svg-sanitisation change) — DOM-based whitelist sanitiser for SVG uploads (script/event-handler/foreignObject stripping, base64 image stripping). The current PR accepts SVG declared type but performs no sanitisation; the serving sibling PR will hard-block SVGs until svg-sanitisation lands.

Files

  • lib/Controller/ResourceController.php — endpoint handler, admin gating, error envelope mapping
  • lib/Controller/ResourceUploadRequestParser.php — JSON body extraction (extracted to keep controller coupling under PHPMD limit)
  • lib/Service/ResourceService.php — base64 parse + size cap + MIME validation + persistence
  • lib/Service/ImageMimeValidator.php — declared-type vs detected-MIME cross-check (uses local error handler instead of @ operator per PHPMD)
  • lib/Exception/* — typed exceptions carrying stable error codes + HTTP status (ResourceException base + 8 concrete subtypes)
  • appinfo/routes.php — one new POST /api/resources route
  • tests/Unit/Service/ImageMimeValidatorTest.php — 6 tests
  • tests/Unit/Service/ResourceServiceTest.php — 11 tests
  • tests/Unit/Controller/ResourceControllerTest.php — 11 tests
  • tests/Unit/Controller/ResourceUploadRequestParserTest.php — 5 tests

Test plan

  • PHPUnit: 33/33 new tests pass; 90 pre-existing tests still pass (123 total) in the Nextcloud Docker container
  • PHPCS: no new violations from this PR's files (4 unrelated pre-existing violations remain in lib/Db/ entity setter calls)
  • PHPMD: no new violations from this PR's files (1 unrelated pre-existing violation in lib/Service/DashboardService.php)
  • Psalm: no new violations from this PR's files (2 unrelated pre-existing violations in PageController and UserAttributeResolver)
  • PHP lint: all new files clean
  • Manual: admin uploads a PNG → file appears in app data, URL returned
  • Manual: non-admin gets 403
  • Manual: 6 MB blob rejected with file_too_large envelope before getimagesizefromstring is reached

Notes for reviewers

  • The success URL is the relative path /apps/mydash/resource/<filename> per the spec scenario, not an absolute URL. The serving endpoint is delivered by the sibling PR.
  • ResourceController::readRequestBody() is protected to allow PHPUnit to inject body bytes (PHP's php://input is not realistically pluggable in unit tests). Production reads from the standard input stream.
  • The Throwable catch in the controller is defence-in-depth: any unexpected exception is logged with detail but the response only contains the safe storage_failure envelope — no raw $e->getMessage() leaks to clients.
  • Pre-existing PHPCS named-arg findings on entity setters were left untouched per project convention — fixing them by adding named args would break Nextcloud's __call magic in Entity.

Spec references

@rubenvdlinde rubenvdlinde added ready-for-code-review Build complete — awaiting code reviewer ready-for-security-review Code review complete — awaiting security reviewer labels Apr 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/mydash @ 14bde87

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-30 11:44 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde rubenvdlinde force-pushed the feature/impl-resource-uploads branch from 7d7510d to 7ce0041 Compare April 30, 2026 12:01
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/mydash @ 4db38ff

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-30 12:02 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde rubenvdlinde force-pushed the feature/impl-resource-uploads branch from 6139e0d to 93cbe3a Compare April 30, 2026 12:05
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/mydash @ f930734

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-30 12:07 UTC

Download the full PDF report from the workflow artifacts.

…RES-001..005

Adds the resource-uploads capability: a small mini file API for
binary branding assets (icons, widget images) that MyDash widgets
need to host directly, separate from the user's Files folder.

- POST /api/resources accepts {base64: 'data:image/<type>;base64,...'}
- Admin-only via IGroupManager::isAdmin (HTTP 403 otherwise)
- Allowed types: jpeg, jpg, png, gif, svg, webp (case-insensitive)
- 5 MB hard cap on decoded bytes, enforced BEFORE invoking the
  image library so oversize blobs cannot be loaded
- Raster MIME cross-check via getimagesizefromstring (SVG validation
  is delegated to the sibling svg-sanitisation capability)
- Storage via IAppData::getFolder('resources') with high-entropy
  resource_<uniqid>.<ext> filenames (folder auto-created on first use)
- Standardised response envelope: success {status, url, name, size};
  error {status, error, message} with stable error enum
- Typed exceptions with stable error codes; raw exception messages
  never leak to clients

Resource serving (GET) and SVG sanitisation are intentionally out
of scope here — they are delivered by sibling changes resource-serving
and svg-sanitisation in their own PRs.

Spec proposal: PR #42 (feature/replica-spec-proposals)
Requirements covered: REQ-RES-001, 002, 003, 004, 005
@rubenvdlinde rubenvdlinde force-pushed the feature/impl-resource-uploads branch from f9191b9 to 324587e Compare April 30, 2026 12:07
@rubenvdlinde rubenvdlinde merged commit e5daf28 into development Apr 30, 2026
31 of 32 checks passed
@rubenvdlinde rubenvdlinde deleted the feature/impl-resource-uploads branch April 30, 2026 12:08
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/mydash @ dcc8bbd

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-30 12:09 UTC

Download the full PDF report from the workflow artifacts.

rubenvdlinde added a commit that referenced this pull request Apr 30, 2026
Add the read side of the resource-uploads capability that complements
the admin-only upload pipeline merged in PR #46:

- Non-OCS GET /apps/mydash/resource/{filename} returning a
  StreamResponse with extension-derived Content-Type (jpeg/png/gif/
  svg+xml/webp; default application/octet-stream) and
  Cache-Control: public, max-age=31536000.
- GET /apps/mydash/api/resources listing endpoint returning
  {status, resources: [{name, url, size, modifiedAt}]} ordered by
  modifiedAt desc; empty folder yields HTTP 200 with [] (never 404).
- Path traversal blocked at routing layer ([^/]+) plus defence-in-depth
  controller checks for .., backslashes, and empty values. All map to
  a uniform HTTP 404 with no detail leaked.
- 50MB+ files (only reachable via manual filesystem tampering) refused
  with HTTP 413 BEFORE bytes are loaded — bounding worst-case memory
  to the 5 MB upload cap from REQ-RES-003.
- Cache-busting via uniqid suffix in REQ-RES-004 filenames — a logical
  asset change yields a brand-new filename, naturally bypassing the
  one-year immutable cache.

Implementation lives in a dedicated ResourceServeController +
ResourceServeService pair so the existing upload controller's
dependency graph stays under the PHPMD CouplingBetweenObjects limit.

Routes registered under the standard `routes` array (mydash currently
has no OCS controller infrastructure); both endpoints carry
@NoAdminRequired so any logged-in user can fetch dashboard assets.

Also fixes a pre-existing PHPStan warning in ImageMimeValidator
(getimagesizefromstring always returns mime, ?? '' was dead).

Tests: 19 new unit tests (8 ResourceServeController, 11
ResourceServeService) covering all 8 spec scenarios — PNG bytes +
headers, SVG content-type, unknown ext fallback, missing file 404,
encoded path traversal 404 (8 vectors via dataProvider), oversize
413 with no read, list ordering desc, empty folder empty array. All
208 PHPUnit tests pass.

Quality: composer check:strict passes (lint, phpcs, phpmd, psalm,
phpstan, test:all). SPDX headers on all new PHP files.
rubenvdlinde added a commit that referenced this pull request Apr 30, 2026
Add the read side of the resource-uploads capability that complements
the admin-only upload pipeline merged in PR #46:

- Non-OCS GET /apps/mydash/resource/{filename} returning a
  StreamResponse with extension-derived Content-Type (jpeg/png/gif/
  svg+xml/webp; default application/octet-stream) and
  Cache-Control: public, max-age=31536000.
- GET /apps/mydash/api/resources listing endpoint returning
  {status, resources: [{name, url, size, modifiedAt}]} ordered by
  modifiedAt desc; empty folder yields HTTP 200 with [] (never 404).
- Path traversal blocked at routing layer ([^/]+) plus defence-in-depth
  controller checks for .., backslashes, and empty values. All map to
  a uniform HTTP 404 with no detail leaked.
- 50MB+ files (only reachable via manual filesystem tampering) refused
  with HTTP 413 BEFORE bytes are loaded — bounding worst-case memory
  to the 5 MB upload cap from REQ-RES-003.
- Cache-busting via uniqid suffix in REQ-RES-004 filenames — a logical
  asset change yields a brand-new filename, naturally bypassing the
  one-year immutable cache.

Implementation lives in a dedicated ResourceServeController +
ResourceServeService pair so the existing upload controller's
dependency graph stays under the PHPMD CouplingBetweenObjects limit.

Routes registered under the standard `routes` array (mydash currently
has no OCS controller infrastructure); both endpoints carry
@NoAdminRequired so any logged-in user can fetch dashboard assets.

Also fixes a pre-existing PHPStan warning in ImageMimeValidator
(getimagesizefromstring always returns mime, ?? '' was dead).

Tests: 19 new unit tests (8 ResourceServeController, 11
ResourceServeService) covering all 8 spec scenarios — PNG bytes +
headers, SVG content-type, unknown ext fallback, missing file 404,
encoded path traversal 404 (8 vectors via dataProvider), oversize
413 with no read, list ordering desc, empty folder empty array. All
208 PHPUnit tests pass.

Quality: composer check:strict passes (lint, phpcs, phpmd, psalm,
phpstan, test:all). SPDX headers on all new PHP files.
rubenvdlinde added a commit that referenced this pull request Apr 30, 2026
Add the read side of the resource-uploads capability that complements
the admin-only upload pipeline merged in PR #46:

- Non-OCS GET /apps/mydash/resource/{filename} returning a
  StreamResponse with extension-derived Content-Type (jpeg/png/gif/
  svg+xml/webp; default application/octet-stream) and
  Cache-Control: public, max-age=31536000.
- GET /apps/mydash/api/resources listing endpoint returning
  {status, resources: [{name, url, size, modifiedAt}]} ordered by
  modifiedAt desc; empty folder yields HTTP 200 with [] (never 404).
- Path traversal blocked at routing layer ([^/]+) plus defence-in-depth
  controller checks for .., backslashes, and empty values. All map to
  a uniform HTTP 404 with no detail leaked.
- 50MB+ files (only reachable via manual filesystem tampering) refused
  with HTTP 413 BEFORE bytes are loaded — bounding worst-case memory
  to the 5 MB upload cap from REQ-RES-003.
- Cache-busting via uniqid suffix in REQ-RES-004 filenames — a logical
  asset change yields a brand-new filename, naturally bypassing the
  one-year immutable cache.

Implementation lives in a dedicated ResourceServeController +
ResourceServeService pair so the existing upload controller's
dependency graph stays under the PHPMD CouplingBetweenObjects limit.

Routes registered under the standard `routes` array (mydash currently
has no OCS controller infrastructure); both endpoints carry
@NoAdminRequired so any logged-in user can fetch dashboard assets.

Also fixes a pre-existing PHPStan warning in ImageMimeValidator
(getimagesizefromstring always returns mime, ?? '' was dead).

Tests: 19 new unit tests (8 ResourceServeController, 11
ResourceServeService) covering all 8 spec scenarios — PNG bytes +
headers, SVG content-type, unknown ext fallback, missing file 404,
encoded path traversal 404 (8 vectors via dataProvider), oversize
413 with no read, list ordering desc, empty folder empty array. All
208 PHPUnit tests pass.

Quality: composer check:strict passes (lint, phpcs, phpmd, psalm,
phpstan, test:all). SPDX headers on all new PHP files.
rubenvdlinde added a commit that referenced this pull request Apr 30, 2026
…53)

Add the read side of the resource-uploads capability that complements
the admin-only upload pipeline merged in PR #46:

- Non-OCS GET /apps/mydash/resource/{filename} returning a
  StreamResponse with extension-derived Content-Type (jpeg/png/gif/
  svg+xml/webp; default application/octet-stream) and
  Cache-Control: public, max-age=31536000.
- GET /apps/mydash/api/resources listing endpoint returning
  {status, resources: [{name, url, size, modifiedAt}]} ordered by
  modifiedAt desc; empty folder yields HTTP 200 with [] (never 404).
- Path traversal blocked at routing layer ([^/]+) plus defence-in-depth
  controller checks for .., backslashes, and empty values. All map to
  a uniform HTTP 404 with no detail leaked.
- 50MB+ files (only reachable via manual filesystem tampering) refused
  with HTTP 413 BEFORE bytes are loaded — bounding worst-case memory
  to the 5 MB upload cap from REQ-RES-003.
- Cache-busting via uniqid suffix in REQ-RES-004 filenames — a logical
  asset change yields a brand-new filename, naturally bypassing the
  one-year immutable cache.

Implementation lives in a dedicated ResourceServeController +
ResourceServeService pair so the existing upload controller's
dependency graph stays under the PHPMD CouplingBetweenObjects limit.

Routes registered under the standard `routes` array (mydash currently
has no OCS controller infrastructure); both endpoints carry
@NoAdminRequired so any logged-in user can fetch dashboard assets.

Also fixes a pre-existing PHPStan warning in ImageMimeValidator
(getimagesizefromstring always returns mime, ?? '' was dead).

Tests: 19 new unit tests (8 ResourceServeController, 11
ResourceServeService) covering all 8 spec scenarios — PNG bytes +
headers, SVG content-type, unknown ext fallback, missing file 404,
encoded path traversal 404 (8 vectors via dataProvider), oversize
413 with no read, list ordering desc, empty folder empty array. All
208 PHPUnit tests pass.

Quality: composer check:strict passes (lint, phpcs, phpmd, psalm,
phpstan, test:all). SPDX headers on all new PHP files.
rubenvdlinde added a commit that referenced this pull request May 3, 2026
…RES-001..005 (#46)

Adds the resource-uploads capability: a small mini file API for
binary branding assets (icons, widget images) that MyDash widgets
need to host directly, separate from the user's Files folder.

- POST /api/resources accepts {base64: 'data:image/<type>;base64,...'}
- Admin-only via IGroupManager::isAdmin (HTTP 403 otherwise)
- Allowed types: jpeg, jpg, png, gif, svg, webp (case-insensitive)
- 5 MB hard cap on decoded bytes, enforced BEFORE invoking the
  image library so oversize blobs cannot be loaded
- Raster MIME cross-check via getimagesizefromstring (SVG validation
  is delegated to the sibling svg-sanitisation capability)
- Storage via IAppData::getFolder('resources') with high-entropy
  resource_<uniqid>.<ext> filenames (folder auto-created on first use)
- Standardised response envelope: success {status, url, name, size};
  error {status, error, message} with stable error enum
- Typed exceptions with stable error codes; raw exception messages
  never leak to clients

Resource serving (GET) and SVG sanitisation are intentionally out
of scope here — they are delivered by sibling changes resource-serving
and svg-sanitisation in their own PRs.

Spec proposal: PR #42 (feature/replica-spec-proposals)
Requirements covered: REQ-RES-001, 002, 003, 004, 005
rubenvdlinde added a commit that referenced this pull request May 3, 2026
…53)

Add the read side of the resource-uploads capability that complements
the admin-only upload pipeline merged in PR #46:

- Non-OCS GET /apps/mydash/resource/{filename} returning a
  StreamResponse with extension-derived Content-Type (jpeg/png/gif/
  svg+xml/webp; default application/octet-stream) and
  Cache-Control: public, max-age=31536000.
- GET /apps/mydash/api/resources listing endpoint returning
  {status, resources: [{name, url, size, modifiedAt}]} ordered by
  modifiedAt desc; empty folder yields HTTP 200 with [] (never 404).
- Path traversal blocked at routing layer ([^/]+) plus defence-in-depth
  controller checks for .., backslashes, and empty values. All map to
  a uniform HTTP 404 with no detail leaked.
- 50MB+ files (only reachable via manual filesystem tampering) refused
  with HTTP 413 BEFORE bytes are loaded — bounding worst-case memory
  to the 5 MB upload cap from REQ-RES-003.
- Cache-busting via uniqid suffix in REQ-RES-004 filenames — a logical
  asset change yields a brand-new filename, naturally bypassing the
  one-year immutable cache.

Implementation lives in a dedicated ResourceServeController +
ResourceServeService pair so the existing upload controller's
dependency graph stays under the PHPMD CouplingBetweenObjects limit.

Routes registered under the standard `routes` array (mydash currently
has no OCS controller infrastructure); both endpoints carry
@NoAdminRequired so any logged-in user can fetch dashboard assets.

Also fixes a pre-existing PHPStan warning in ImageMimeValidator
(getimagesizefromstring always returns mime, ?? '' was dead).

Tests: 19 new unit tests (8 ResourceServeController, 11
ResourceServeService) covering all 8 spec scenarios — PNG bytes +
headers, SVG content-type, unknown ext fallback, missing file 404,
encoded path traversal 404 (8 vectors via dataProvider), oversize
413 with no read, list ordering desc, empty folder empty array. All
208 PHPUnit tests pass.

Quality: composer check:strict passes (lint, phpcs, phpmd, psalm,
phpstan, test:all). SPDX headers on all new PHP files.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

ready-for-code-review Build complete — awaiting code reviewer ready-for-security-review Code review complete — awaiting security reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant