Context
internal/web/handlers.go::handleAddItem resolves a parent entity by ID to get its FullPathDisplay, then calls App.CreateEntity with ParentPath (which re-resolves that path inside its own transaction). If a rename or reparent occurs between the two reads, the second resolution may bind to a different entity than intended, or fail.
parent, err := s.cfg.App.GetEntityByID(r.Context(), parentID) // read 1: parent path snapshot
// ... another request renames parent here ...
s.cfg.App.CreateEntity(ctx, CreateEntityRequest{ParentPath: parent.FullPathDisplay}) // resolves stale path
The same TOCTOU pattern exists in every web POST handler that holds an ID but the App API requires a path: RenameEntity, ChangeStatus (via handleToggleMissing), and CreateEntity (via handleAddItem).
Impact
For the current single-user, home-LAN deployment, the practical risk is negligible — only one person is moving things around. If the web UI ever gets multi-user concurrent access, this becomes a real correctness problem.
Proposed fix
Add ID-based write APIs to internal/app:
CreateEntityUnderID(ctx, CreateUnderRequest{ParentID, ...})
RenameEntityByID(ctx, RenameByIDRequest{EntityID, ...})
ChangeStatusByID(ctx, ChangeStatusByIDRequest{EntityID, ...})
These would resolve the entity once inside a transaction and dispatch the event without re-resolving a path. Existing path-based APIs stay for CLI ergonomics.
References
- Identified during web-package security/robustness review.
- Related code smell: web handlers also do extra
ListEntities scans to translate IDs↔paths, which an ID-based API would obviate.
Context
internal/web/handlers.go::handleAddItemresolves a parent entity by ID to get itsFullPathDisplay, then callsApp.CreateEntitywithParentPath(which re-resolves that path inside its own transaction). If a rename or reparent occurs between the two reads, the second resolution may bind to a different entity than intended, or fail.The same TOCTOU pattern exists in every web POST handler that holds an ID but the
AppAPI requires a path:RenameEntity,ChangeStatus(viahandleToggleMissing), andCreateEntity(viahandleAddItem).Impact
For the current single-user, home-LAN deployment, the practical risk is negligible — only one person is moving things around. If the web UI ever gets multi-user concurrent access, this becomes a real correctness problem.
Proposed fix
Add ID-based write APIs to
internal/app:CreateEntityUnderID(ctx, CreateUnderRequest{ParentID, ...})RenameEntityByID(ctx, RenameByIDRequest{EntityID, ...})ChangeStatusByID(ctx, ChangeStatusByIDRequest{EntityID, ...})These would resolve the entity once inside a transaction and dispatch the event without re-resolving a path. Existing path-based APIs stay for CLI ergonomics.
References
ListEntitiesscans to translate IDs↔paths, which an ID-based API would obviate.