File migration reports success when rclone fails

[oc-tmueller]

Summary

StateMigrateFiles::cloneFilesForUser() ignores rclone's exit code. The only failure detector is
the output line processor, which scans each line for EMERGENCY, ALERT, CRITICAL, ERROR,
WARNING or NOTICE. If rclone fails in a way that prints none of those tokens, the migration is
reported as successful even though not a single file was transferred, and the state machine
advances to StateMigrateShares.

How this was found

This is the reason #63 shipped unnoticed. With --insecure off, rclone refused to run at all:

Command sync needs 2 arguments maximum: you provided 3 non flag arguments: ["" "oc-demo,type=webdav,…" "ocis-migrate,type=webdav,…"]

That message contains none of the six tokens, so $verified stayed true, cloneFilesForUser()
returned true, no MigrateException was raised, and doMigrate() moved on to share migration.
An administrator running the documented procedure was told the file migration had succeeded while
their users' data had not been copied. #63/#68 fix the argument list, but not the reporting.

Why it matters beyond #63

Any future rclone invocation failure behaves the same way: a missing or non-executable
bin/rclone_linux_amd64, an unrecognised flag after a binary bump, an OOM kill, a non-zero exit
with output on stdout only. For a one-way data migration, silently claiming success is the worst
possible failure mode — the admin's next step is to decommission the source instance.

Suggested fix

Consult Process::isSuccessful() after run(), in addition to the line scan:

$process = new Process($cmd);
$process->setTimeout(null);
$process->run($lp);
$lp->close();

if (!$process->isSuccessful()) {
    $conflictLogFile->putCSV(['ERROR', $user->getUserName(), $user->getEMailAddress(), "rclone exited with {$process->getExitCode()}"]);
    $verified = false;
}

Worth considering at the same time:

• The token scan is substring-based and case-sensitive, so a path or filename containing the word
  ERROR fails a user's migration spuriously, while a genuine lowercase failure passes.
• tests/acceptance/migrate/migrate.sh only ever runs migrate:to-ocis:init -k -f, so the
  trusted-certificate path has no end-to-end coverage at all. An acceptance run without -k would
  have caught File migration fails when --insecure is false #63.

Scope note

Deliberately left out of the 3.0.2 patch release: the change makes migrations that currently
report success start failing, which deserves its own review rather than riding along with a
bugfix.

🤖 Generated with Claude Code