feat(factory-reset): add wipe modes 2 and 3 by JanZachmann · Pull Request #25 · omnect/omnect-os-init

JanZachmann · 2026-09-18T13:53:21Z

Summary

Adds the factory-reset wipe modes 2 and 3. Mode 2 overwrites etc and data with data from /dev/urandom, mode 3 discards every block of both with the BLKDISCARD ioctl. The wipe runs between the backup and the reformat and never aborts the reset: a failure on one partition does not skip the other, reformat and restore still run, and the outcome is reported as Error with the reason in error. Mode 4 is no longer an accepted mode.

It also closes what the on-device tests found: a trigger the init cannot parse is answered instead of ignored, and a preserve configuration that says nothing is rejected instead of skipped. A blank trigger is not one of those — it is no reset request at all.

Design: docs/superpowers/specs/2026-08-11-factory-reset-modes-2-3-4-design.md, with a correction of what it claimed about rejected triggers.

Reason

The legacy shell init implemented the wipe with external tools. Both are replaced by native code, so the behaviour differs in ways worth stating.

Mode 2 covers the whole partition from byte 0. The legacy dd started at offset 2048 so an interrupted wipe kept the volume label readable; the init finds partitions by number, never by label, and re-creates the filesystem anyway. Privacy is the same or better. The legacy dd also used oflag=direct; the write loop goes through the page cache instead and flushes every gigabyte and again at the end, so after a power loss at most one gigabyte of already overwritten blocks can still hold the old content.

Mode 3 issues one BLKDISCARD over the whole partition. The legacy path mounted it, removed the files and ran fstrim, which trims only the blocks the remove had freed and leaves journal remnants — and counted a failed mount as a successful wipe. Without discard support the ioctl now fails cleanly and the reset reports it. Discard stays a hint on some disks, so the "no total privacy guarantee" note in the layer README remains true.

Both modes open the device with O_EXCL, so a partition that is still mounted fails with EBUSY rather than being overwritten under a live filesystem.

Mode 4 is dropped, not ported. It handed the wipe — the one step the caller asked for — to a customer-supplied hook that is not built or tested here, and modes 2 and 3 cover rotating disks and flash. The removal has to land in the same release wherever the mode is known: the mode table in the meta layer README, and the mode type in omnect/omnect-device-service#207 and omnect/omnect-ui#128.

A failed wipe reports Error, not Warning: the device is usable, but the operation the caller asked for did not happen. For modes 2 and 3 the destructive boundary moves to the wipe, so every outcome from there on reports data_wiped — a partly written overwrite destroys the data even when the reformat never runs.

A trigger the init cannot use is now answered. It used to produce a kmsg warning only: the boot continued as Normal, no factory_reset object was written, and the trigger kept its value, so the warning repeated on every boot while the caller waited for a result. Such a trigger is now cleared like any other and reported with the status the shell implementation used — anything about mode is Invalid, anything about the preserve configuration is ConfigError. That also makes preserve mandatory again, and a file in factory-reset.d without a usable paths array an error rather than something to skip, since skipping it wipes the paths the caller asked to keep and still reports success.

A blank trigger is not a reset request. GRUB reports a key set to an empty value as present where U-Boot reports it as unset, and the shell init cleared the key by writing an empty string. A device coming from it would otherwise be answered with one "reset failed" nobody asked for.

The statuses a configuration problem reports are now the same wherever it occurs: a factory-reset.json that does not exist reports ConfigError like a key missing inside it, which is what the shell implementation did too. Only an absent file counts — a path that exists but cannot be read stays an I/O error and reports Error, because the request was usable and the storage was not, which is worth retrying. The error field itself has one shape everywhere now: it sits inside the factory-reset result, so no writer repeats the outer Factory reset error: prefix.

Known gap. Nothing pins that the wipe runs after the backup and after the unmount — moving wipe_and_rebuild above backup_all passes the whole suite. The wipe_and_rebuild seam starts after both steps, so covering the order against them needs a mount/backup seam as well. Tracked separately, together with the data_dev / etc_dev argument swap, which has the same shape.

The result schema is unchanged and the initramfs image needs no new packages: the wipe uses one ioctl and /dev/urandom instead of fstrim, blkdiscard or dd.

Mode 2 overwrites etc and data with random data from /dev/urandom, mode 3 discards all their blocks with the BLKDISCARD ioctl. The wipe runs between the backup and the reformat and never aborts the reset: a failure on one partition does not skip the other, reformat and restore still run, and the result is reported as Error with a note in the status error field. Legacy mode 4 (customer-supplied wipe hook) is not ported; a mode-4 trigger stays rejected as Invalid. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

The note is built from the error's Display, which prefixes "Factory reset error" - inside the factory-reset result that only repeats the obvious. Take the inner reason instead. The overwrite test only checked that the last block changed, so a gap in the middle of the range would have passed. Check every block. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

…an ioctl Seeking to the end of a block device reports its size, which drops one raw ioctl request code, its unsafe block and the size_t reasoning that came with it — and it works on a temp file, so the size path is testable. The remaining BLKDISCARD code is assembled from named constants. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

Taking the device size from a seek made both testable against a temp file, so the tests now exercise wipe_random itself instead of only its inner loop, including the error a device it cannot open produces, and that the size path rewinds for the caller that writes from the start. The progress interval becomes an argument of the overwrite loop. At its 1 GiB default no test ever crosses it, which left the sync and step arithmetic uncovered. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

A trigger that failed to parse only produced a kmsg warning. The boot continued as Normal, no factory_reset object was written and the trigger kept its value, so the caller waited for a result that never came and the warning repeated on every following boot. The on-device tests catch this: they set broken triggers and expect a status. The trigger is now carried into factory-reset mode either way. An unusable one skips the sequence, is cleared like any other, and is reported with the status the shell implementation used: anything about mode is Invalid, an unusable preserve is ConfigError. preserve becomes mandatory again - an empty array is how a caller asks to keep nothing. The design of the wipe modes describes the reporting as if it already happened, so it carries a correction. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

A file in factory-reset.d without a usable paths array was skipped, and a broken factory-reset.json was reported as an invalid request. Both are configuration errors: the caller asked to keep something and the file does not say what, so skipping it wipes those paths and still reports success. Every problem in a preserve configuration file now reports ConfigError, which is what the shell implementation did and what the on-device tests expect. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

Deduplication: the same get/as_array/as_str block existed three times after the trigger parsing was rewritten. It moves into a helper that reports why a value was not a list of strings, so each caller keeps its own status - a missing key in the trigger is a different answer than one in a preserve file. WipeFailed was built at four call sites with the same two fields. Tests: three of them passed even with the behaviour removed. The overwrite tests accepted a constant buffer, so mode 2 now checks that two wipes differ and that a block carries many distinct values; the rejected-trigger tests built the error themselves instead of walking detect; and the integration test accepted any mode instead of the one the trigger named. Mode 3 had no test at all - both of its failure paths work on a regular file. The 32-bit clamp becomes a function so the 4 GiB case is a table test instead of a comment. The two new error variants get a recovery-class test, since classifying either as fatal would block a boot the design says must continue. Docs: the design of the wipe modes still described the size ioctl that the seek replaced, and named a constant that was renamed; the README claimed a value has to be valid json to reach factory-reset mode; the module map had no factory_reset subtree. The two older factory-reset specs carry a dated correction, the form this repo uses for superseded statements. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

init_setup runs the sync on every boot, and its failure suppresses the first-boot marker like a failed resize does. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

…tion Every command in the build chapter omitted the partition table, which build.rs demands, so none of them worked. The overview counted the shell scripts the binary replaced; the count says nothing once the migration is over. The startup flow now also names the extra-bootargs update as a reason to reboot, next to the filesystem check. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

JoergZeidler

Ran fmt, clippy (grub,gpt and uboot,dos, each with factory-reset,test-utils), tests for five combinations including release-image, cargo audit and the armv7 clippy from the README: all green.

Points without a diff line:

Spec status line still says "In review (PR #23)".
Spec 2.3: the example note "wipe of data failed: <reason>" does not match the {partition}: {reason} the code produces.
Spec section 4 assigns the meta-omnect README rewrite to omnect/meta-omnect#636. That PR is closed without a successor, and the README on feature_init_rust still describes dd, rm + fstrim and the custom-wipe hook. Where does that land?
The mode-4 removal exists as omnect/omnect-device-service#207 and omnect/omnect-ui#128. Worth linking from the description, since the spec asks for the same release.
PR text: the legacy dd used oflag=direct, the new loop writes through the page cache. Spec 1.1 asks for every delta to be listed.
project-context.md still lists src/preflight/; the directory is src/init_setup/.
mod.rs:31: the CONTEXT_SEPARATOR doc lists two uses, there are four now. mod.rs:34 contrasts with mount_points.
tests/factory_reset.rs:1: "detect fallback" is stale.

Signed-off-by: Joerg Zeidler 62105035+JoergZeidler@users.noreply.github.com

Drop the doc duplicates the review found: the testability rationale kept only on ReformatRetryOps, the const docs that restate their name, the first lines that restate a match or a min, and the sentences that document another function. Reword the CONTEXT_SEPARATOR doc for its four uses, the flush note, and the discard test's error mapping, and say in chunk_len_clamps_without_truncating that the 32-bit lint run is the real guard. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

`/etc/omnect/factory-reset.d/` is always preserved, so an empty `preserve` array keeps that directory, not nothing — fix the claim in the README, the spec and the parse doc. Also state that the directory is only read when `preserve` contains `applications`, that a trigger which is not json at all reports status 1, and that U-Boot builds exist for both partition tables; name the 32-bit rule instead of a machine. project-context.md had drifted from the tree (a directory that was renamed, a claim about tmpfs, a claim about the test combinations) and duplicates the README. Untrack it and add it to .gitignore. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

…atuses A blank trigger is no longer a reset request. GRUB reports a key set to an empty value as present where U-Boot reports it as unset, and the shell init cleared the key by writing an empty string, so a device coming from it would otherwise get one unasked-for "reset failed" report. A missing factory-reset.json now reports ConfigError like a missing key inside it, and a factory-reset.d file without a `paths` key uses MissingField like the other two callers, so the same problem never reports two different statuses. `FactoryResetTrigger::Rejected` and `WipeOps` carry `FactoryResetError` instead of `InitramfsError`, which is the only thing either can hold; the wipe note drops the match that unwrapped it. `failure_status_code` keeps its catch-all arm because it also classifies `run_reset` failures, which really can be any `InitramfsError`. A mode that is numeric but not a usable mode number no longer reports "no numeric 'mode'". The wipe opens the device with O_EXCL, so a mounted partition fails with EBUSY instead of being overwritten. The mode is checked once above the wipe loop, and the factory_reset integration test target declares the feature it is gated on. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

`run` split into `answer_trigger`, so the branch this PR adds is testable: a trigger the init cannot use must be cleared and recorded, and must not start a reset. A `run` that forgot either would have passed before. `run_reset` split into `wipe_and_rebuild` with an injected `WipeOps`, so the order is pinned: the wipe runs before the reformat, mode 1 skips it, and a wipe failure reaches the status even when the rebuild succeeded. Also cover a write failure inside the overwrite loop, assert the wipe note really dropped the outer error prefix, and drop the wipe-status test for a path `wipe_partitions` cannot produce. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

The doc still said a set key dispatches to factory reset whatever its value; a blank one now falls back to Normal. Also keep the spec status at "In review" — the number was the stale part, not the state. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

JoergZeidler

Round 2 on bf3a715. fmt, clippy for grub,gpt / uboot,dos / grub,dos / uboot,gpt (each with factory-reset,test-utils), tests for five combinations including release-image, cargo audit and the armv7 clippy run: all green. Resolved the round-1 threads that are fixed; the two you declined (mkfs guard, catch-all arm) are fine with me and resolved as well.

One point I would not merge over:

project-context.md deleted and gitignored (10b920f). The template system says the opposite: setup-ai.sh help text calls it "Source of Truth: check in project-context.md to git", and the omnect-ai-templates README lists it as tracked in the project. After this PR no tracked file carries the BootMode recipe or the planned-work list, and the generated CLAUDE.md points to §8 twice. Drift is a reason to fix the file, not to drop it. Left the two project-context.md threads open for that.

Worth one sentence in the description: the backup-before-wipe order in run_reset has no test (moving wipe_and_rebuild above backup_all passes everything). Pinning it needs a mount/backup seam, so fine to leave as a known gap.

Signed-off-by: Joerg Zeidler 62105035+JoergZeidler@users.noreply.github.com

Dropping the file was wrong: setup-ai.sh names it as the source of truth to check in, and it is the only tracked input to the generated AI context files, which are untracked by design. Section 8 also carries the BootMode recipe, which lives in no other tracked file. Fix what had drifted instead: the renamed init_setup directory, the backup that sits in initramfs RAM rather than on a tmpfs, the test-matrix sentence that claimed the README lists every factory-reset combination, and section 8 still listing FactoryReset as a planned variant. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

… untested The status `error` field carried two shapes: a rejected trigger kept the outer "Factory reset error:" wrapper while a wipe note dropped it. One `error_text` helper now renders every writer the same way. Three things had no test and could be broken silently: the signal `wipe_and_rebuild` hands back, the mode-to-function mapping in `RealWipeOps`, and the non-NotFound branch of the factory-reset.json read. The set_env-failure case used a mock holding no trigger, so it passed even if the clear never ran; it now seeds one and asserts it survives, and an accepted trigger with a failing clear pins that the reset still runs. Only an absent factory-reset.json reports ConfigError. Any other read failure stays an I/O error, because the request was usable and the storage was not, which is worth retrying. Also: reuse `WipeResult` instead of a function-local alias, bind the rebuild closure before the call, fold the second wipe-ops mock into the first, and drop the comments that name the previous implementation. The 32-bit note on `chunk_len_clamps_without_truncating` no longer claims the armv7 clippy run guards it — that lint cannot tell the cast orders apart. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

JoergZeidler

Round 3 on 0c82c3f. Everything from rounds 1 and 2 is in, checked against the diff; all threads resolved. fmt, clippy for the four base combos with factory-reset,test-utils, tests for five combinations including release-image, cargo audit and the armv7 clippy run: all green.

Merge-ready from my side.

A few low points left, your call whether to take them in this PR or not:

mod.rs:106 / mod.rs:475: the prefix strip in error_text is not pinned by a test; both consumers only assert error.is_some(). Reverting to e.to_string() keeps the suite green.
mod.rs:94: the error_text doc says every writer produces the same shape. The other arm still writes Filesystem error: ... for a mount failure from run_reset. Not a bug, the subsystem prefix is useful there, but the doc overclaims; "only the FactoryReset wrapper is stripped" is what the code does.
project-context.md: the module tree omits src/recovery.rs; cargo build / cargo check without features fail (build.rs needs a bootloader feature), pre-existing line; §3 says the factory_reset test needs the feature but lists none of the four factory-reset combinations from the README.
mod.rs:1186: the test comment says "the only link from ResetMode to the real functions"; the test pins the two RealWipeOps bodies, the mode mapping lives in wipe_partitions and is covered by the mode2/mode3 tests.

Signed-off-by: Joerg Zeidler 62105035+JoergZeidler@users.noreply.github.com

JoergZeidler

looks good to me, see round 3 comment.

The prefix strip in `error_text` had no test: reverting it to `e.to_string()` kept all 352 tests green, because both consumers only assert that an error is present. Two tests now cover it — the `FactoryReset` wrapper is dropped from both writers of the field, and another subsystem keeps its prefix, which is what says where the failure came from. The doc claimed every writer produces the same shape; it strips one wrapper. `real_ops_call_the_wipe_that_matches_the_method` is what the test does. It pins the two `RealWipeOps` bodies; which method a mode picks lives in `wipe_partitions` and is covered by the mode2/mode3 tests. project-context.md: add recovery.rs to the module tree, say that a build needs a bootloader and a partition table since build.rs rejects anything else, and list the four factory-reset test combinations the README has. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

The note said devices carry an empty value without saying how they get one or why grub is named. Both backends can be asked to clear a trigger by writing an empty value; fw_printenv then reports the variable as unset while grub-editenv still lists the key, so only on grub does a cleared trigger come back as present but empty. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

data and etc travelled as two adjacent &Path arguments, so swapping them at a call site wiped etc as data and left every test green - the tests pass their own paths and cannot see the mix-up. A struct with named fields moves the mistake to where it would be written. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

omnect#25 landed squashed, so the merge base is still the flash spec commit and git re-offered every file both branches touched. The content was already here from the earlier merge of the branch; the one real change is the last review round on omnect#25, which replaced the two `&Path` arguments of `wipe_partitions` and `wipe_and_rebuild` with a `WipeTargets` struct. Everything else resolves to this branch: the flash module and its dispatch, `reformat_ext4` in `crate::filesystem`, `detect` taking `&mut dyn BootEnv`, and the README and project-context entries for the flash features. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

JanZachmann added 8 commits September 18, 2026 15:20

docs(factory-reset): drop comments the code already says

873f18e

Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

JanZachmann marked this pull request as ready for review September 21, 2026 09:37

JanZachmann requested review from JoergZeidler and mlilien September 21, 2026 09:37

JanZachmann added 2 commits September 21, 2026 13:58

docs(readme): name the extra-bootargs step in the startup flow

42a0f38

init_setup runs the sync on every boot, and its failure suppresses the first-boot marker like a failed resize does. Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>

JanZachmann mentioned this pull request Sep 21, 2026

feat(flash): port flash mode 1 to the Rust initramfs #27

Merged

JoergZeidler reviewed Sep 22, 2026

View reviewed changes

JanZachmann added 5 commits September 22, 2026 18:10

JoergZeidler reviewed Sep 23, 2026

View reviewed changes

JoergZeidler requested changes Sep 23, 2026

View reviewed changes

JanZachmann added 2 commits September 23, 2026 08:54

JanZachmann requested a review from JoergZeidler September 23, 2026 07:40

JoergZeidler reviewed Sep 23, 2026

View reviewed changes

JoergZeidler approved these changes Sep 23, 2026

View reviewed changes

mlilien reviewed Sep 23, 2026

View reviewed changes

Comment thread README.md Resolved

JanZachmann requested a review from mlilien September 23, 2026 15:56

mlilien approved these changes Sep 23, 2026

View reviewed changes

JanZachmann merged commit 9344a2c into omnect:main Sep 24, 2026

JanZachmann deleted the feat/wipe-modes-2-3 branch October 23, 2026 06:56

Conversation

JanZachmann commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Reason

Uh oh!

JoergZeidler left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JoergZeidler left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JoergZeidler left a comment

Choose a reason for hiding this comment

Uh oh!

JoergZeidler left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JanZachmann commented Sep 18, 2026 •

edited

Loading