fix(security): guard eml-preview with an explicit file-access check

[rubenvdlinde]

Closes #408.

What this changes

EmlPreviewController::preview renders the original, un-redacted EML for a caller-supplied fileId and had no ownership or permission check of its own. This adds verifyFileAccess() — the same guard AnonymizationController already applies to the sibling extract / anonymize endpoints under the same api/anonymization/ URL prefix — plus 5 unit tests.

Why, given cross-user access is already denied

Two agents disagreed today over whether this was an exploitable IDOR. A live two-user probe settled it: unguarded, but not currently exploitable. The denial is real, but it belongs to Nextcloud, not to DocuDesk.

preview($fileId)
  -> EmlPreviewService::renderOriginalPreview($fileId)
    -> OpenRegister FileService::getFileById($fileId)
      -> IRootFolder::getById($fileId)     <- ROOT scope, not the user folder
      -> checkOwnership() == $node->isReadable(), whose own docblock says the
         owner-vs-session comparison is deliberately omitted

IRootFolder::getById() is Folder::getById() → Root::getByIdInPath($id, ''). With an empty path the $user derived from the path is null, so the mount cache is queried across all users, and the hits are then intersected with the current session's mounts. That intersection is the only thing between a caller and another user's message — and it disappears the moment the same code runs without a session user: a background job, an occ command, any system-context call.

Live probe (shared dev instance, NC 34.0.0, two fresh non-admin users, Basic auth, no shared cookie jar, OCS-APIRequest: true)

Caller	fileId	Owner	HTTP	Body
(unauthenticated)	21992	A	401	Current user is not logged in
B	21991	B (own)	422	... requires the OpenRegister anonymise-EML API.
B	21992	A (cross-user)	422	... file 21992 is not a readable file node.
B	99999999	(no such file)	422	... file 99999999 is not a readable file node.
B	21992	A, shared to B	422	... requires the OpenRegister anonymise-EML API.
B	21992	A, share removed	422	... file 21992 is not a readable file node.

renderOriginalPreview() resolves the file before it checks for the OpenRegister API, so the two error strings distinguish the two steps exactly. Rows 5 and 6 are the differential control: same user, same id, same request — grant a share and resolution succeeds, remove it and it fails again. The gate is precisely mount/share visibility, not authorisation.

Strict subset

getUserFolder($uid)->getById() still resolves shared files (shares are mounted inside the user folder), so the shared-file case above stays a grant. The guard denies exactly what is already denied, only explicitly and early, and keeps denying if resolution ever leaves a session. 404 rather than 403 so callers cannot probe for existence.

Verification

• phpunit tests/unit/Controller/EmlPreviewControllerTest.php → OK (5 tests, 13 assertions)
• Negative control: with the verifyFileAccess() call removed and everything else identical → 3 of 5 fail (401→422, 404→422 ×2). The tests can fail.
• Wider suite EmlPreviewControllerTest + EmlPreviewServiceTest + AnonymizationControllerTest → OK (32 tests, 78 assertions)
• phpcs --standard=phpcs.xml lib/Controller/EmlPreviewController.php → exit 0 (2 pre-existing @spec warnings, present before this change)
• phpstan analyse lib/Controller/EmlPreviewController.php → [OK] No errors

Out of scope, flagged in #408

anonymizeEmlStructured does not exist in OpenRegister — not in the deployed 0.2.17-unstable.25, not on origin/development, not on any origin/* branch (verified with a positive control: getFileById is found by the same query). Every call to this endpoint therefore 422s at the method_exists() check, and src/services/fileViewerService.js:82 points the file viewer at it. The feature is dead end-to-end; that is a separate fix.

[github-actions]

Quality Report — ConductionNL/docudesk @ 7dee9da

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
test		✅
test-unit		✅
check-manifest		✅
test-l10n		✅
composer			✅	✅ 114/114
npm			✅	✅ 646/646
PHPUnit					✅
Newman					✅
Playwright					✅
Hydra gates					❌

---

Quality workflow — 2026-08-09 14:05 UTC

Download the full PDF report from the workflow artifacts.