fix(toolkit): keep source DB intact when DbMove copy fails - #6946

Great fix for the copy-failure case. One adjacent gap worth flagging while you're in here: FileUtils.deleteDir (plugins utils/FileUtils.java:73-84) still recurses with java.io.File, and isDirectory() follows symlinks — so a directory symlink gets traversed and its target's contents are deleted.

This composes with this PR: after db mv, the original path is a symlink to the moved DB (DbMove.java:176). If a later db lite merge deletes the archive dbs under that original path (DbLite.backupArchiveDbs calls FileUtils.deleteDir per db), it wipes the already-migrated DB on the new disk — two legitimate operations, chained data loss, no attacker needed.

Suggested fix: replace the recursion with Files.walkFileTree + NOFOLLOW_LINKS (delete the link itself, never follow). The copy phase already uses NOFOLLOW_LINKS (:154/:198), so this would make the delete side consistent. The same pattern also exists in common/.../FileUtil.java:114-126 and the toolkit copy in tron-docker's tools/toolkit — happy to split that into a separate PR if you prefer.

Thanks for flagging this. The symlink traversal issue is valid, but it predates this PR: db mv already created symlinks, and this change does not modify FileUtils.deleteDir or the db lite merge workflow.

I’ll keep this PR focused on preventing source deletion after copy failures and reporting migration failures correctly. The deletion behavior and lite merge’s handling of relocated databases should be addressed in a separate PR, with regression coverage for the combined workflow.

[MUST] Please reject destinations inside any selected source before copying. With configuration order B, A and A's destination under B's source, B is copied before A's destination exists. Finalizing B deletes A's copy; finalizing A then deletes its original, leaving a dangling link while returning 0. I reproduced this regression: the same configuration preserves A's data on the base. Cross-check the canonical paths with destination.startsWith(source) before creating any destination, return 2 for unsupported layouts, and add self-nesting and cross-database regression tests.

Confirmed, and it is order-dependent on the base as well. Same layout, both configuration orders:

A destination inside any selected source is now rejected up front with exit 2, regardless of order (9ec2584). Added self-nesting and cross-database tests covering both orders.

I'll update the design writeup in #6940 accordingly. It previously listed this layout as a non-goal, leaving the outcome to the operator and outside the guarantees; it is now rejected before anything is written.

[SHOULD] A runtime exception during a destination write, such as SecurityException under a SecurityManager, escapes this handler and bypasses rollback. I reproduced exit 1 with all sources intact but completed and partial destinations left behind; retry after removing the denial returns 2. Please handle expected filesystem runtime failures inside the workers and during destination creation/traversal, then roll back after all workers have finished. An outer finally alone does not ensure outstanding parallel copies have stopped.

Fixed in e6daa90: the copy phase now handles runtime exceptions like I/O errors. Workers catch them, so the parallel forEach only returns after every worker has finished, and destination creation (Files.createDirectories) moved into the same handler; rollback runs afterwards. Same repro now: exit 1, no leftovers, and the unchanged retry returns 0.

I did not add a regression test for this path: raising a RuntimeException from the JDK file APIs here requires a SecurityManager, which is disabled by default since JDK 17 and removed in JDK 24. The shared failure path is covered by the existing I/O-failure tests.

[SHOULD] This fails the first configured database, so it does not verify rollback after an earlier database has copied successfully. Please add a later-database failure case asserting unchanged source contents, removal of all created destinations, and successful retry. For cleanup failure, assert that all sources remain intact and leftover destinations are reported. For link-creation failure after source deletion, assert that the complete destination is retained and recovery instructions are emitted. Both failures should return non-zero and omit the success message.

Added in f3deb19.

[NIT] Please use plain db in the help text, since the backticks are printed literally. Consider including the stop-the-node note in db mv -h as well; this parent-command header is not shown in the subcommand's help.

Done in 5d35137. The note is now a shared constant (Db.STOP_NODE_HEADER) used as the header of both db and db mv, with plain db instead of backticks. db mv -h now starts with:

All db tools operate directly on the database files.
Before performing a database operation,
you must stop the currently running FullNode service.