[FIX] dms: scope directory share tokens to the token's own subtree

[Hotdgo9]

check_access_token in dms/models/dms_file.py walks up the directory tree to decide whether the token's directory is the file's own directory or one of its ancestors — but both comparisons test the walker against itself rather than against the token's directory (item). At 98f4088, L181 and L185:

directory_item = self.directory_id
while directory_item.parent_id:
    if directory_item.id == self.directory_id.id:   # true on the first iteration
        return True
    directory_item = directory_item.parent_id
# Fix last level
if directory_item.id == self.directory_id.id:       # also always true
    return True

On the first iteration directory_item is self.directory_id, so it returns immediately; and if the directory has no parent the loop body never runs, at which point the "fix last level" line makes the same always-true comparison. The whole if items: block reduces to return True — the token's own directory is never consulted. Stated once: the effect is that any valid directory share token authorises read of every dms.file in the database, irrespective of directory or group inheritance.

dms.directory.check_access_token (models/directory.py L241) does the same walk correctly, comparing against item.id at L251/L257/L261 — the intended logic is already in the tree next door.

The portal hunk is mandatory, not optional

_get_files (controllers/portal.py L162) calls check_access_token on an empty dms.file recordset at L187, where self.directory_id.id is False. The loop is skipped and the always-true False == False on the last-level line is currently what makes shared-folder listings work at all.

Fix the model alone and every shared folder renders empty: HTTP 200, no traceback, nothing in the log. So the two hunks have to land together. The token belongs to a directory, so it is validated against the directory being browsed — exactly what _get_directories (L193) already does. _get_files returns early when dms_directory_id is falsy, so the browse() is always on a real id.

Bounded walk

The ancestor walk is unbounded in both files. Before this fix the file-side loop always returned on iteration one, so it never actually traversed; once it does, a parent_id cycle would spin indefinitely on a request that needs no authentication. _check_directory_recursion rejects cycles created through the ORM, so realistically only corrupted data reaches it, but the seen set costs nothing.

Tests

8 regression tests in dms/tests/test_access_token.py — unrelated trees, root-level files, descendant directories, a file's own token, and the shared-folder listing the portal hunk protects. Red before, green after.

Scope

This block is byte-identical on 12.0 through 18.0 and dates to the 13.0 migration, so the released branches carry it too. Landing it here means 19.0 never ships it. I am opening separate PRs against 17.0 and 18.0, where the patch applies verbatim; 16.0 and older need a hand-port because _get_files/_get_directories do not exist there.

Disclosure

Being straightforward: the root cause and impact are already described in the commit body on our public fork. We found this auditing our own production instance and fixed it there before working out how to raise it upstream, and OCA/dms has private vulnerability reporting disabled — so by the time we got here there was no quiet route left. Flagging it rather than leaving you to find it. Our own instance had never created a share token, so we were never exposed.

Happy for you to take these commits under your own name if that keeps the migration moving.

Assisted-by: Claude Opus 5