unidoc isms lets an empty folder be deleted

The issue, in one line

A folder made in the Documents tree could never be removed again, because there was no delete route and the only way out was editing the bare git repo by hand. The issue.

The cause

Folder creation got an endpoint, a role check and a UI action. Deletion got none of them. Documents each had their own delete route, so nothing covered the container, and an empty folder stayed in the tree forever. The manual workaround, committing to the bare repo, skipped the app, the commit author and the activity log, which is the opposite of what a compliance product wants in its audit trail.

Where it lives

• internal/isms/store/git.go, DeleteDirectory took the store lock and removed whatever path it was handed, with nothing in it that tested the folder for documents first
• internal/isms/api/server.go, no handler existed for a folder delete, so there was no route, no role check and no validation of the path
• internal/isms/api/openapi.yaml, the documents paths listed folder creation and document deletion but no folder delete
• web/src/views/Documents.vue, the folder context menu had no Delete entry at all

The fix, planned in the free diagnosis

• Add DELETE /api/v1/documents/folders?path=... for managers and admins, the roles that can already create a folder, and document it in openapi.yaml. A path with an empty, . or .. segment returns 400 and a missing folder returns 404.
• Add Store.DeleteEmptyFolder, which walks the tree recursively, treats a folder as empty when every file under it is a .title file, and returns ErrFolderNotEmpty as 409 without moving HEAD when a document is present.
• Split the commit code into deleteDirectoryUnlocked so the emptiness check and the removal run inside one hold of the store mutex, which stops a document committed into the folder mid-delete from being removed along with it.
• Commit the removal as the acting user with a “delete folder” commit message, matching the create commit, and write folder_deleted to the activity log.
• Show Delete in the folder menu only when the recursive file count is zero, behind a confirmation step, reusing the existing locale keys.

How it’s proven, in the sprint

• internal/isms/store/git_test.go, TestDeleteEmptyFolder. An empty folder that holds an empty subfolder is deleted by a commit authored by the acting user. A folder with a document one level down is refused and HEAD does not move. A missing path and a file path both return not found.
• internal/isms/api/folder_delete_pg_test.go, run with ISMS_TEST_DATABASE_URL. A folder created with POST /documents/folders is deleted, disappears from /documents/all, carries the acting user as commit author and leaves a folder_deleted activity row. The same test pins the refusals, 409 for a folder with a document, 404 for a missing folder, 400 for path traversal, 403 for the contributor and reader roles.
• tests/test_documents.py, three API tests that fail against a server built from master and pass against the branch.
• gofmt, go vet ./... and go test ./internal/... clean with and without a database, pytest 802 passed and 1 skipped, npm test 257 passed, npm run build clean, and a browser check that Delete appears only on an empty folder and removes it after confirmation.
• The approving reviewer did not rely on that write-up. They read the full diff and the go-git source behind the one claim that could not be taken on faith, ran go build, go vet and gofmt, ran the Go suite against a live PostgreSQL with ISMS_TEST_DATABASE_URL set and again with it unset, ran npm test at 257 and npm run build, and then mutation-tested the emptiness check and the traversal guard. With the traversal check removed, the inner_traversal test really did delete an unrelated folder through path collapsing, and the test caught it.

What wasn’t tested, said up front

The repository reports no CI check runs on the head or merge commit, so the counts are local runs quoted from the PR body. The reviewer reproduced the build, vet, gofmt, the Go suite with and without a live PostgreSQL and both JS commands, then mutation-tested the two guards, but had no browser or pytest in their sandbox, so the Python count and the browser walkthrough rest on the author and on the reviewer reading the tests. A document committed into the folder during the delete is argued safe by the single mutex hold rather than by a test that races the two. The stale activeFolder highlight the reviewer found is cosmetic and was left to a follow-up. No load test on a deep tree.

What happened next, after the sprint

It was merged on 2026-10-03 about 13 hours after it opened, with 489 lines added and 0 removed. The reviewer wrote “mutation-tested both pieces of logic that matter most for a delete endpoint” (source). Open the pull request.