migrate-profile: hardening, and the defects device testing found - #146
Draft
Yury-MonZon wants to merge 13 commits into
Draft
migrate-profile: hardening, and the defects device testing found#146Yury-MonZon wants to merge 13 commits into
Yury-MonZon wants to merge 13 commits into
Conversation
This was referenced Jul 29, 2026
Yury-MonZon
force-pushed
the
migrate-tool-improvements
branch
from
July 29, 2026 17:32
8c57dc7 to
9849fd9
Compare
Yury-MonZon
force-pushed
the
migrate-tool-improvements
branch
from
July 31, 2026 08:53
9849fd9 to
c0634b2
Compare
Yury-MonZon
force-pushed
the
migrate-tool-improvements
branch
2 times, most recently
from
July 31, 2026 10:04
30b0dbe to
328dea0
Compare
Yury-MonZon
force-pushed
the
migrate-tool-improvements
branch
from
July 31, 2026 10:47
328dea0 to
047ad21
Compare
Yury-MonZon
force-pushed
the
migrate-tool-improvements
branch
from
July 31, 2026 12:59
047ad21 to
c65824e
Compare
merge_accountdb replaced /etc/passwd, /etc/shadow, /etc/group and /etc/gshadow with whatever the merge printed as long as awk exited 0, and fell back to src's copy verbatim when it did not. Both are load-bearing: an empty or truncated result is a profile that cannot be logged into, and taking src's copy wholesale drops the service accounts the newer base and the apt replay just created. Validate before replacing: non-empty, a root entry present, every record carrying the right number of colon-separated fields, and no duplicate keys. The field count is what catches truncation, which the root check alone does not. On rejection keep dst's file, leave src's beside it as .migrate-theirs and record a conflict line. src's copy is now taken verbatim only when dst has no such database at all. The four databases are also merged one file at a time, and each merge only runs if that file shows up in the delta, so nothing guaranteed the results agreed with each other: a passwd entry with no shadow entry is an account that cannot authenticate, and group drifting from gshadow is what grpck exists to complain about. Compare the key sets of each pair once merging and deletions are done. Drop origin_base_build from the origin marker while here. It was written and never read: the stock name carries the build id (@Desktop_744_stock), so origin_stock_name already pins the exact base, and btrfs send -p refuses a parent that is not a real ancestor.
…db.awk The 55-line merge was a heredoc written to $WORK on every run, so it could not be syntax-checked, diffed or tested on its own. Ship it as a file and run it with awk -f, so mawk -W dump validates it like any other source. A missing file is rejected alongside the other checks rather than silently taking one side.
Packages were replayed before any file was merged, so a package installed from a repository the user added was never findable: dst still had the old /etc/apt, and it ended up reported as "install by hand" although its source was sitting in src. Merge etc/apt and usr/share/keyrings first, then replay, then merge the rest. Keyrings also needed exempting from the usr/ denylist, since a source without its key is no better than no source.
An unresolved .migrate-conflict or .migrate-theirs sidecar counted as a user change, so a second migration would carry it forward as if the user had authored it. .migrate-bak, the resolv.conf moved aside for the apt replay, has the same problem when a run is interrupted.
A package the user installed by hand is in no repository, so the replay could only report it and move on, and its files are not carried either since usr/ is denylisted precisely because packages are meant to be replayed. But the .deb itself is often still on disk: in src's apt cache if apt-get downloaded it, or wherever the user put it. @home and @root are shared subvolumes, so a .deb sitting there is already reachable from dst. Collect every .deb under those roots in one traversal, with no depth limit since a file below an arbitrary cutoff would be reported as missing when it is not, and verify a candidate with dpkg-deb rather than trusting its filename: the Package must match and the Architecture must be ours or all. The version src had is preferred, any verified version is the fallback. Install it with apt-get so dependencies still resolve from the repositories, and report which packages were recovered, at which version, and which are still missing.
The preview classified anything whose base copy is not a regular file as a both-sides overlap, so the symlinks the delta always carries (bin, lib, sbin, var/run) were reported as needing a 3-way merge: a run with one real conflict announced five. merge_one takes them wholesale, never merging, so classify them the way it treats them.
`btrfs subvolume snapshot ... && echo ... || die ...` also dies when the snapshot succeeded and only the echo failed, and it reads as if die guarded the snapshot alone. if/then/else, so losing the rollback point is the only thing that aborts.
The header claimed the migrated profile keeps the device's ssh identity, but nothing in the code arranged that, and on real builds it did not happen. Caught by the cross-build run on the device: migrating a 774 profile onto a 775 base left the destination with 775's host keys, so booting it would have greeted the user with a host-key-changed warning. The reason is that the build bakes the keys into the image, so the source profile's copy is byte-identical to its _stock ancestor. The delta ancestor -> src therefore never mentions them, and the generic file merge cannot carry what it is never told about. Copying them explicitly after the file pass fixes it: 6 key files carried, and the destination's fingerprint now matches the device. Worth noting separately: baking host keys into the image means every device flashed from the same build shares an ssh identity, and reflashing changes it. Deleting /etc/ssh/ssh_host_* at the end of the build would let sshd-keygen generate per-device keys on first boot, which is a change to image behaviour rather than to this tool.
Two cases are settled by policy rather than left to the user: a file the user deleted that the new base also changed (the new version wins) and a directory the user deleted that the new base still fills. Both were appended to the conflict list, so the summary counted them as needing a decision and said they had been left as sidecars, when no sidecar exists and --resolve would never show them. The cross-build run made it visible: 4 decisions claimed against 3 sidecars on disk. They now go to a separate notices list, counted and listed apart, so the decision count matches what --resolve offers. The account-database disagreement note moved with them for the same reason: it reports a state, not a choice. Verified on the device, where the counts now agree and the deleted-and-changed file is reported under "settled for you".
WORK was created near the top of the script, but the trap that removes it can only be installed
after mount_top, because mount_top installs an EXIT trap of its own and would clobber ours. Any
die inside mount_top therefore ran top_cleanup, which knows nothing about WORK, and the
directory stayed behind.
In normal use that window is invisible. In a recovery boot it is the common case: / is a RAM
rootfs, so every invocation that forgets -d dies exactly there and leaks another directory.
Found on the device booted into recoveryOS.
Creating WORK immediately after the trap closes the window by construction, rather than adding
a second cleanup channel to keep in sync. mig_cleanup guards on ${WORK:-} so it stays safe if it
ever runs before the assignment.
Removing the work directory is an AND-list, so a failing rm ends the trap and becomes the script's exit status. Nothing follows it today, but the shared cleanup path no longer leaves that to chance.
collect_debs walked three roots with [ -d "$_dir" ] && find ..., so find exiting non-zero, which it does when anything under @home or @root is unreadable or disappears mid-walk, ended the whole run under set -e. That scan is best-effort by nature: fewer candidates only means a package gets reported as missing instead of recovered. Verified by running the function as written against a find that fails on the second root: before, the caller never reaches the result; after, the scan continues and still finds the .deb under @home. Measured on the development host, since migrate-profile is not carried to recoveryOS.
Yury-MonZon
force-pushed
the
migrate-tool-improvements
branch
from
July 31, 2026 15:15
c65824e to
513d853
Compare
… from Under -d the header read exactly like a local run, so nothing distinguished migrating profiles on another filesystem from migrating your own. list-profiles already names the filesystem it lists; this does the same, and only when the two differ.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The migration tool, kept last so it can be reviewed on its own. Ten commits, and apart from one
stamp_stock_origintidy it touches onlymigrate-profileand its awk helper.Hardening
non-empty output. A rejected merge keeps the destination's file and leaves a sidecar plus a conflict
line. An earlier version of this guard accepted a truncated
shadow, which is what the field-countcheck closes.
/usr/lib/flipper-accountdb.awk, verified byte-identical tothe heredoc it replaced and clean under
mawk -W dump, since the device has mawk rather than gawk.repository the user added cannot be resolved at all.
migration.
.deb, matched on packagename and architecture through
dpkg-deb -frather than on filename.Defects found by testing
build bakes the keys in, so the source's copy is identical to its ancestor's and the delta never
mentions them; a migration therefore changed the device's ssh identity. Now carried explicitly, driven
by one helper that both the preview and the apply read. Verified by booting the migrated profile: a
client holding the pre-migration keys connects under
StrictHostKeyChecking=yeswith no warning.as needing a decision, so the summary said four while three sidecars existed, and pointed at
--resolvefor a file it would never show. They are now counted apart, under "settled for you".mount_topdied, because the trap that removes it can only beinstalled after
mount_topinstalls its own. One leak per invocation in a recovery boot.&& echo ... || die, which also dies when thesnapshot succeeded and only the echo failed.
Testing. Migration ran across two real builds from different commits, so both-sides conflicts had
somewhere to go, and the load-bearing case passed: a package installed from a repository present only on
the source, which the destination can resolve only if apt configuration merged first. Every
--resolvemenu key was driven through a pty against sidecars a real migration produced. A user account added before
the migration logs in afterwards with its carried password hash.
Split out of the closed #141. Only the combined final state was run on hardware; each
commit parses and defines the helpers it uses, and the stack tip is byte-identical to what
was tested.