Skip to content

toyos-metal: a bench's disks are known by what they hold, not by their node - #1064

Merged
Japabu merged 4 commits into
mainfrom
metal-stick-port
Oct 27, 2026
Merged

Japabu merged 4 commits into
mainfrom
metal-stick-port

Conversation

@Japabu

@Japabu Japabu commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

The metal loop wrote /dev/sda unless --device said otherwise, and its only admission check was size, vendor, model and removable. The T14 now has a second SanDisk Ultra of the same model, holding an EndeavourOS ISO, and it answers all four checks. If enumeration put that stick at sda, the loop would wipe it. Neither the node nor the USB port is stable: sticks move between ports.

The owner has since given each bench's toyos account passwordless sudo. Disk selection is therefore the only thing that stops a wrong-disk write, and the narrow-root machinery has no remaining purpose.

Resulting behaviour

Prerequisite: the bench account has passwordless sudo. The loop checks this with sudo -n /usr/bin/true and refuses (Sudo) without it.

One rule, located (tests/metal/src/lib.rs), selects the boot stick. It reads one lsblk --pairs listing and selects the one /dev/sd<letter> disk that both answers the recorded identity and carries the boot stick's mark. If no disk matches, or more than one does, the loop refuses (Unlocated / Ambiguous) and names every disk it considered, with that disk's partition types. It never falls back to a node.

Role Mark Used by
boot stick, before a flash a ToyOS image's table: one ESP, one TOYOS-LOG, one slot table, 1–2 BOOT and 1–2 ROOT partitions (admit holds every image to the same table, so a flashed stick is always found again) flash, boot entry
boot stick, after the boot this image's TOYOS-LOG partition GUID (drawn fresh per build) the stick wait, mount, raw read: they follow the stick to whatever node it enumerated as
scratch disk exactly one partition, of the new type Guid::TOYOS_SCRATCH (51CC09F9-7442-435B-BC03-0FD8718B0FB5); the mark is the whole of its identity --adopt-scratch now; destructive tests later

The marks exclude each other. Automatic selection can therefore never take one stick of a model for another of the same model, in either role, and never takes a blank or foreign disk. That guarantee is located's. It does not cover a disk named by hand with --device, or the window described under risks.

Actions, each of its own (beside only --host/--key, plus --dry-run for adoption):

  • --record /dev/sd<letter> records a machine with that disk as its boot stick, or checks the disk against an existing record. This is the record-writing half of the old --install-sudoers.
  • --adopt-scratch /dev/sd<letter> refuses a disk that carries a ToyOS image or is not removable. Otherwise it wipes the disk, writes a one-partition scratch table, and reads the mark back.
  • --device /dev/sd<letter>, on a boot, adopts a boot stick that has no ToyOS image yet. The identity is checked; the content check is skipped.

Deleted, everything that existed only to grant narrow root:

  • --install-sudoers, the password pipe, the staged rule, visudo -c/install, and sudo -n -l over every line;
  • the rendered rule: the jobs! table, Mask, its fills and samples, Word and its sudoers escaping, user_word;
  • the Word, PartitionIndex and Rule refusals, and the dry run's per-line sudo question (a dry run now prints the lines);
  • the fixed ESP/LOG partition numbers the rule named (the image's own numbers are used);
  • issues/the-sudoers-rendering-has-no-host-side-judge.md, and the host-tool row for staging the rule.

Job now carries what it acts on (Flash(Sd), Mount { sd, part }, Retag(Vec<(Owned, Guid)>), …), and Target::argv is still the one place a root line is written.

Size against main: lib.rs production is 2569 → 2515 lines (−54 net, with disk selection and adoption added); tests are 1226 → 1262. The rule removal alone was −363 production and −167 test lines.

Unchanged:

  • The internal install. It writes nvme0n1, which no USB or card reader takes; it is recognised by its layout and refused if removable.
  • Boot entries. They are matched on the ESP's fresh partition GUID, and the collision check keeps that GUID off every other disk.
  • The record format. It is the same as main's.

Checks and evidence

  • cargo test -p toyos-metal: 82 passed.
  • Negative controls m1–m6, each applied as a checked patch, built, run and reverted. All exit 101 at the named test:
    • layout check removed;
    • first match wins;
    • log GUID ignored;
    • scratch mark accepts extra partitions;
    • adoption's image guard removed;
    • admit's BOOT count removed.
  • A real staged metal image is still admitted by the new admit (the dry run reaches the ssh step).
  • located and adoptable ran against both benches' real unprivileged listings (lsblk 2.39.3): sda is selected on each; MBR types (0x0/0xef/0xc) carry no mark; mmcblk0 and nvme0n1 are excluded. This still holds: selection is unchanged apart from Layout dropping the fixed numbers, and the bench tables carry one of each part regardless.
  • cargo run -- --ci host: see the latest comment.
  • Metal, done. In both T14 runs the driver printed disk /dev/sda: … carrying a ToyOS image and the boot stick enumerated as /dev/sda, and listed every sudo -n line. Driver logs: driver/.
    • T14 at 477c14c: flash, boot, readback; acpi_server_death 1/1.
    • T14 at a732584: flash, boot, readback; acpi_server_death 1/1.
    • AMD dry runs at a732584 (image and --adopt-scratch /dev/sdb): both exit 0, /dev/sda selected by content, nothing written.
    • The one change since a732584 refuses an image before the flash and does not touch selection.

Unresolved risks

  • The one item no bench has exercised: sgdisk --clear after wipefs on a real adoption. Only dry runs have run adoption so far. The PARTUUID follow after the boot is settled by both T14 runs.
  • Locate→write window. Between located and wipefs/dd, a disk replaced under the same letter would be written. This needs a hot-plug, or both sticks re-enumerating with swapped letters, within those seconds. Writing through /dev/disk/by-diskseq/N, which is never reused, would close it. The sudoers obstacle is gone with the rule, but no bench reading has confirmed the by-diskseq links yet, so this is recorded, not built.
  • Root is unrestricted. With passwordless sudo for everything, a defect in this tool's command construction is no longer stopped by sudo. Target::argv is the one place a root line is written, and no_root_line_writes_the_benchs_own_partitions covers the internal disk.

🤖 Generated with Claude Code

…r node

The loop wrote /dev/sda unless --device said otherwise, and admitted it by
size, vendor, model and removable alone. A second SanDisk Ultra on the T14,
holding a live ISO, answers all four, so an enumeration that put it at sda
would have been wiped. Neither the node nor the USB port is stable: sticks
move between ports.

One rule, `located`, now finds every disk a bench lends: the one
/dev/sd<letter> disk that answers the recorded identity *and* carries what
its role is marked by, out of one `lsblk --pairs` listing. None or more than
one is refused, naming every disk considered; nothing falls back to a node.

- The boot stick before a flash carries a ToyOS image's table: the ESP and
  TOYOS-LOG at the rule's numbers, one slot table, one or two slots and
  ROOTs. After the boot it is the disk carrying this image's own TOYOS-LOG
  GUID, wherever it enumerated, and the mount and raw read follow it.
- A scratch disk, which destructive tests may erase, carries exactly one
  partition, of a new type `Guid::TOYOS_SCRATCH`. The two marks exclude each
  other, so neither role is ever taken for the other, even between two sticks
  of one model.

Adoption is explicit and names the node by hand. A boot stick with no ToyOS
image yet is adopted with --device /dev/sd<letter> (identity checked, content
check skipped); a capture with no record needs --device too.
--adopt-scratch /dev/sd<letter> refuses a disk carrying a ToyOS image, wipes
the named disk, writes the scratch table, finds it again through `located`
and adds its identity to the record's new `scratch` list. The record's
`Stick` becomes `Disk`, shared by both roles; it is shaped so a card behind an
SD host controller can be recorded later, which is not built.

The sudoers rule names the stick as /dev/sd[a-z] (`Mask::Sd`, filled with the
located letter), since the letter is enumeration's. It still names no NVMe
disk, and no partition of one outside the install area. Each bench's rule has
to be reinstalled with --install-sudoers.

The internal install is unaffected: it writes nvme0n1, which no USB or card
reader takes, recognised by its layout and refused if removable. Boot entries
are matched on the ESP's freshly drawn partition GUID, which the collision
check keeps off every other disk.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu

Japabu commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

r1 checks at ba94b33

  • cargo run -- --ci host: exit 0, 103 steps green. log
  • Negative controls m1–m5 (patch + log each), all exit 101 at the named test: r1/
  • An earlier host run at an intermediate tree was red for two reasons: a Record initializer in tests/checks/metal.rs (fixed in this commit), and a doctest that lost toyos_build while I was running cargo alongside it. The doctest passes alone. Neither failure applies to the run above.

Bench commands, in order (the owner runs these)

  1. Read-only, on each bench. This prints no serial and no partition GUIDs:
ssh -i ~/.ssh/id_ed25519_toyos_runner -o IdentitiesOnly=yes -o BatchMode=yes toyos@bench-t14 'lsblk --version; lsblk --noheadings --paths --pairs --output NAME,TYPE,PKNAME,PARTN,PARTTYPE,RM,SIZE; for d in /sys/class/block/sd? /sys/class/block/mmcblk?; do [ -e "$d" ] || continue; echo "== $d"; cat "$d/size" "$d/removable"; cat "$d/device/vendor" "$d/device/model" 2>/dev/null; cat "$d/device/manfid" "$d/device/name" "$d/device/type" 2>/dev/null; done'

On the AMD, use the same command with its user@host. The output confirms four things: PKNAME is a path; PARTTYPE is readable without root; which node carries the ToyOS table; and that each disk's sysfs identity matches the record. It also shows what the microSD answers, for the future record shape.
2. Reinstall the rule (writes /etc/sudoers.d/toyos-metal). The new rule still permits main's /dev/sda lines, so it can go in before landing:
cargo run -p toyos-metal -- --install-sudoers <password file>, then the same with --host <user>@<amd>.
3. Metal run on the T14, end to end (locate → flash → boot → follow the log GUID → mount and raw read): T14 RUN REQUESTED, request prepared from this worktree at ba94b33.
4. After landing only. Adopt the scratch disks. Use the node letters step 1 shows at that moment; adoption refuses any disk carrying a ToyOS image.

  • Dry run: cargo run -p toyos-metal -- --adopt-scratch /dev/sdb --dry-run (T14), and cargo run -p toyos-metal -- --adopt-scratch /dev/sdc --dry-run --host <user>@<amd>
  • Then the same two commands without --dry-run. This erases both disks.
  • It has to wait for landing because the record gains a scratch list, and main's reader (deny_unknown_fields) would refuse that record.
  • The T14 microSD (mmcblk0) is not adoptable yet, by design.

@Japabu

Japabu commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

located checked against both benches' listings (lsblk 2.39.3, read without root). Evidence: bench/. The test was a throwaway, applied and reverted, and is not committed. No code change is needed.

  • T14, boot stick: sda (ESP, SLOTS, LOG, BOOT, ROOT) is the one boot stick. Only sda is asked for its identity. sdb (the ISO) is never asked.
  • ISO partition types: the MBR PARTTYPEs 0x0/0xef on sdb read as no type, because Guid::parse accepts only 36-character GUIDs.
  • Excluded disks: mmcblk0 and nvme0n1 are not /dev/sd<letter>, so the listing skips them.
  • T14, scratch: with no disk adopted yet, the scratch lookup refuses and names both sd disks.
  • T14, adoption: adoptable refuses sda (it carries a ToyOS image) and admits sdb.
  • AMD: sda is the boot stick. The trimmed sysfs vendor and model, USB / SanDisk 3.2Gen1, match the record. The Transcend reader is sdb, not sdc, so step 4's AMD command is --adopt-scratch /dev/sdb --host <user>@<amd>.

Still open for the metal run: the listing was taken without PARTUUID. The after-boot lookup reads PARTUUID from the same udev record as PARTTYPE, and the T14 run exercises it.

@Japabu

Japabu commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

Review r1 — #1064 at ba94b33

Architecture

The design is right. located is now the single rule every writer and reader of a bench disk goes through. It selects a disk by what the disk holds, and it has no node default. It fails closed on zero matches and on several. It replaces both the /dev/sda default and the old identity-only check, and it adds no second path. Adoption is explicit. The flash, boot entry, wait, mount and raw read all take the Sd that located returned.

I traced the cases the brief asked about:

  • Enumeration flip or port move. The Layout mark selects the right stick. After the boot, the stick is followed by its fresh TOYOS-LOG GUID. A stick that never came back is now NoStick → Stick. Before this PR that case wrote /dev/sda, which could be the ISO.
  • Half-written flash. wipefs runs, then dd. If dd dies before its first 4 MiB block, the stick is left blank and the next run refuses until --device adopts it. If dd dies after that block, the new primary GPT is in place, Layout matches the same stick, and the next run reflashes it. Both outcomes are correct.
  • Two ToyOS tables. Two sticks of the same identity are refused as Sticks. A ToyOS stick of another model fails the identity check and is never written.
  • MBR / ISO. PARTTYPE="0x0"/"0xef"/"0xc" and an MBR PARTUUID fail Guid::parse, which requires exactly 36 characters (lib/gpt/src/guid.rs:110). So the ISO and the FAT32 card carry no mark.
  • mmcblk0 and nvme0n1. Sd::parse excludes both before any read. The internal-install path is unchanged.
  • Adoption. adoptable refuses a disk that carries a Layout, and one that is not listed. The removable check refuses a fixed disk. A bench's two identical SanDisks are told apart by Layout versus no Layout, which is the right fact.
  • Sudoers. Mask::Sd renders [a-z] and fills exactly one ASCII lower-case letter (lib.rs:~705). The rule has no *, so no argument can absorb a space or a suffix. /dev/sd[a-z]3 matches one partition node, and NVMe is still named only by the internal lines. On both benches today every sd disk is USB (bench/{t14,amd}.txt: internal disks are nvme0n1 and mmcblk0). In practice the widening admits the other USB disk.
    • The old /dev/sda rule never protected against that: the old rule already allowed writing the ISO after a flip.
    • Which USB disk gets written is now decided by the client alone. A future bench with an internal SATA disk would be writable through this rule, guarded only by the client's removable check. That is acceptable as long as the PR says so.
  • Identifiers. No serial or partition GUID from a bench is committed. Test GUIDs are synthetic. The bench listing in evidence omits PARTUUID.

Size: roughly +300 production and +245 test lines in lib.rs. That is proportionate for one selector, one adoption action and the rule plumbing for a filled letter. The one piece that does not earn its place is the record's scratch list (B2).

Negative controls m1–m5 are real mutations: m1 is true || …, m2 accepts the first match, m3 accepts any PARTUUID, m4 accepts any scratch partition, and m5 drops the image guard. Each fails the named test.

  • m1–m3 ran on an earlier tree with 86 tests. The mutated lines are byte-identical at the head, so the evidence still holds.
  • Host CI ran at ba94b33 with exit 0.

Findings

BLOCKER B1 — tests/metal/src/timings.rs:92-95 — the sentence starting "A card behind an SD host controller would answer its manfid and name … reading one is not built" should be deleted.

  • It documents a capability that does not exist, which makes it design speculation rather than a contract.
  • The PR body already says the microSD is out of scope. The rest of the doc comment can stay.

BLOCKER B2 — tests/metal/src/timings.rs:110-114, tests/metal/src/lib.rs:2189-2192 — Record::scratch is written and never read.

  • Nothing in the PR reads record.scratch. Adoption verifies itself with the disk it has just read, not with the record.
  • The mark alone identifies a scratch disk: only --adopt-scratch writes TOYOS_SCRATCH, and a scratch disk is erasable by definition. A recorded identity therefore protects nothing that the mark and the removable check do not already protect.
  • The planned destructive tests run in ToyOS and find the disk by its GPT type, not through the host's record.
  • The list costs:
    • a field with a serde default;
    • a round-trip test;
    • the operational rule "adopt only after landing, or main's deny_unknown_fields refuses the record".
  • Fix: drop the field and its test, and let adoption confirm Carrying::Scratch on sd. The alternative is to name the concrete reader that needs the identity.

BLOCKER B3 — evidence — the end-to-end path has not run on metal.

  • No measurement has shown the following:
    • that the PARTUUID-based follow after the boot works (the bench listing omitted PARTUUID);
    • that the [a-z] rule passes visudo -c and sudo -n -l with the filled argv;
    • that the mount and the raw read work with a filled letter.
  • The PR already plans this. The cheapest sufficient evidence is:
    • reinstall the rule on both benches;
    • run the requested T14 run (acpi_server_death) to completion;
    • run one AMD --dry-run through the new rule, so that both benches' rules are known to work before the nightly matrix depends on them.
  • Attach the logs.

SUGGESTION S1 — lib.rs:2319 → flash — the window between located and wipefs/dd should be recorded in the PR's risks. The PR currently says two sticks "can never be taken for each other".

  • A disk replaced under the same letter during those seconds would be written.
  • In practice this needs a human hot-plug, or both sticks re-enumerating so that the letters swap. The kernel reuses the lowest free index.
  • Closing it fully means naming the disk by something that is never reused, such as diskseq (/dev/disk/by-diskseq/N), at the cost of the rule's /dev/sd restriction. Recording it is enough for this PR.

SUGGESTION S2 — lib.rs:149-151,190 — the variants Stick (the stick did not come back), Sticks (several disks match) and NoStick (no disk matches) are three unrelated failures with near-identical names.

  • Sticks and NoStick also apply to scratch disks.
  • Names such as Unlocated / Ambiguous would read unambiguously at their match sites, for example in wait_for_the_stick.

SUGGESTION S3 — lib.rs:2176 — answered.check(&disk) compares the identity with itself, so only the removable check can fail. It reads like a no-op, yet it is the only guard against adopting a fixed disk. Say removable directly.

SUGGESTION S4 — lib.rs:585 — when NoStick is reported after a failed flash, both T14 sticks read "does not carry a ToyOS image's partition table". The operator then has to choose --device between two identical sticks, which is the original hazard moved into a human's hands. Adding each disk's partition count (for example "2 MBR partitions" against "none") to considered makes the choice safe.

SUGGESTION S5 — lib.rs:569-575 — trim located's doc to its contract: the one disk that carries the mark and answers want, refused when there are none or several. The list of examples and "The one rule for every disk a bench lends, and for every writer of one" repeat the module header.

PR description (for the orchestrator). Two corrections:

  • "Not yet confirmed: … PKNAME as a path and PARTTYPE without root" is stale. The second PR comment and bench/ confirm both, so only PARTUUID remains open.
  • "Two sticks of one model can therefore never be taken for each other" is true of located, not of --device or of the TOCTOU window (S1).

SEND BACK

… disk's table

- The record's scratch list is gone: nothing read it. A scratch disk's
  identity is the one TOYOS_SCRATCH partition only adoption writes, and what
  carries it is erasable by definition. Adoption reads the mark back off the
  named disk instead of recording it. `Stick` keeps its name, since the
  boot stick is the one disk a record names.
- A disk a refusal considered is named with its partitions' types, so the
  operator choosing --device between two sticks of one identity can see
  which one is which.
- `NoStick`/`Sticks` are `Unlocated`/`Ambiguous`, since they say the same of
  a scratch disk.
- Adoption checks `removable` directly rather than checking an identity
  against itself.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu

Japabu commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

r2 at 477c14c: review r1 response

  • B1 fixed: timings.rs is back to main's version. The SD-card sentence and the Stick→Disk rename are both gone.
  • B2 fixed: the record's scratch list and its test are removed. A scratch disk's identity is its mark. Adoption reads Carrying::Scratch back off the named disk. This removes the "adopt only after landing" constraint.
  • B3: the coordinator runs this. The commands are below.
  • S1: taken as a recorded risk in the PR body. diskseq is supported by the 6.8 kernels, but sudoers cannot mask a number of variable length without *, so it is not cheap.
  • S2: NoStick/Sticks are renamed Unlocated/Ambiguous.
  • S3: adoption checks removable directly.
  • S4: each considered disk is named with its partition types, e.g. (2 partitions: no GPT type, no GPT type) or (no partitions).
  • S5: located's doc is trimmed to its contract.
  • PR body: the stale line is fixed, and the "never confused" claim is scoped to automatic selection.

Checks:

  • cargo run -- --ci host: exit 0, 103 green.
  • cargo test -p toyos-metal: 89 pass.
  • m1–m5 rerun at this tree; each exits 101 at its named test.
  • Evidence: r2/
  • The bench-listing check from r1 still holds: located, Carrying::holds and sd_disks are unchanged except for the refusal text.

Bench commands (B3), run from this worktree at 477c14c:

  1. Reinstall the rule. The owner's password is read from the file named in the command:
    • T14: cargo run -p toyos-metal -- --install-sudoers <password file>
    • AMD: cargo run -p toyos-metal -- --install-sudoers <password file> --host <user>@<amd>
  2. T14 run: the prepared request (acpi_server_death, staged at 477c14c).
  3. AMD dry runs (they write nothing):
    • cargo run -p toyos-metal -- --image <worktree's request image: metal-t14/acpicase/image.img> --dry-run --host <user>@<amd>. This locates the stick, then asks sudo -n -l about every line it would run.
    • cargo run -p toyos-metal -- --adopt-scratch /dev/sdb --dry-run --host <user>@<amd>. On the AMD, sdb is the Transcend reader.

…ule goes

The owner gave each bench's account passwordless sudo for everything
(/etc/sudoers.d/90-toyos-nopasswd). What existed only to grant and check a
narrow root is deleted:

- `--install-sudoers`, the password on a pipe, the staged rule and its
  `visudo -c`/`install`, and `sudo -n -l` over every line;
- the rendered rule: the `jobs!` table, `Mask` and its fills and samples,
  `Word` and its sudoers escaping, `user_word`;
- the `Word`, `PartitionIndex` and `Rule` refusals, and the dry run's asking
  sudo whether the rule permits a line (a dry run now prints the lines);
- the fixed ESP and TOYOS-LOG numbers the rule named: a ToyOS table is one
  ESP, one TOYOS-LOG and one slot table wherever they sit, and the mount, raw
  read and boot entry take the image's own numbers;
- issues/the-sudoers-rendering-has-no-host-side-judge.md, and the host-tool
  row for staging the rule.

`Job` now carries what it acts on (`Flash(Sd)`, `Mount { sd, part }`,
`Retag(Vec<(Owned, Guid)>)`, ...), and `Target::argv` is still the one place
a root line is written. Capture, the record-writing half of the old install,
is `--record /dev/sd<letter>`, an action of its own like `--adopt-scratch`.

With sudo allowing anything, disk selection (`located`) is what stops a
wrong-disk write, and it is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu

Japabu commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

r3 at a732584: the narrow sudo rule removed (owner's direction; the bench account has passwordless sudo)

Deleted, all of it machinery for granting and checking narrow root:

  • --install-sudoers, the password pipe, the staged rule (visudo -c/install), and sudo -n -l over every job;
  • the jobs! table, Mask with its fills and samples, Word with its sudoers escaping, and user_word;
  • the Word, PartitionIndex and Rule refusals, and the dry run's per-line sudo question;
  • the fixed ESP/LOG numbers the rule named;
  • six rule-only tests: rule rendering, sudoers escaping, account-name widening, mask fills for boot ids and GUIDs, and the rule/argv one-table test;
  • issues/the-sudoers-rendering-has-no-host-side-judge.md (35 lines), and the host-tool row for staging the rule.

Line counts:

  • lib.rs against r2: production 2878 → 2515 (−363), tests 1429 → 1262 (−167).
  • lib.rs against main: production −54 net, even with disk selection and adoption added.
  • Whole commit: +227/−757 in lib.rs.

Kept:

  • located and adoption, unchanged.
  • A typed Job carrying its own values, with Target::argv as the one place a root line is written.
  • require_sudo, which now refuses with "the bench account needs passwordless sudo".
  • Capture is now --record /dev/sd<letter>, an action of its own like --adopt-scratch.
  • Docs: README, tests/CLAUDE.md and --help (generated from the flags) never described bench setup, so they needed no edit. The prerequisite is stated in the module header, the Sudo refusal, and issues/machines-anyone-can-lend-to-toyos.md.

Checks and evidence:

  • cargo run -- --ci host: exit 0, 103 green.
  • m1–m5 rerun at this tree; each exits 101 at its named test.
  • Evidence: r3/
  • The T14 run at 477c14c passed: flash, boot, the stick found by content, then readback. acpi_server_death 1/1. Masked readback: t14-477c14c21/. Selection and the after-boot GUID follow are unchanged since, apart from Layout dropping the fixed numbers, so that run still evidences selection.

Commands at a732584 (from this worktree):

  1. T14 boot: the request is re-staged at metal-t14-r3/request.txt (acpi_server_death).
  2. AMD dry runs (they write nothing):
    • cargo run -p toyos-metal -- --image <metal-t14-r3/acpicase/image.img> --dry-run --host <user>@<amd>
    • cargo run -p toyos-metal -- --adopt-scratch /dev/sdb --dry-run --host <user>@<amd>
  3. Optional cleanup on each bench: the old rule file is now dead. ssh <user>@<bench> sudo -n rm /etc/sudoers.d/toyos-metal

@Japabu

Japabu commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

Review r2 — #1064 at a732584

Architecture

Deleting the narrow-root machinery is the right response to the owner's direction. Its only purpose was to grant and check a restricted sudo, and with passwordless sudo none of it guards anything. What replaces it is smaller and easier to follow:

  • Job is now a typed enum that carries what it acts on.
  • Target::argv is the one function that builds a root command line.
  • require_sudo refuses when there is no passwordless sudo.

Disk selection (located) is unchanged from r2.

Size: against main, production in lib.rs is −54 net, even with disk selection and adoption added. Tests are +36. The commit since r2 is −363 production and −167 test lines, plus a 35-line issue. Nothing kept is speculative.

The deletion is complete.

  • I searched the tree, excluding rust/ and the issue archive, for sudoers, install-sudoers, visudo, sudo -n -l, Mask, Word, user_word, PartitionIndex, staged, esp_part/log_part and "the rule". The only hit left is the stale line in B1 below.
  • Each removed refusal (Word, PartitionIndex, Rule) is gone from the enum, from Display and from the tests.
  • No dependency is orphaned. Every [dependencies] entry is still used, and the import of std::io::Write was dropped.
  • The harness (tests/toyos.rs, build/) never passed --install-sudoers or relied on --device for capture.
  • Every instruction reference is updated: the help text comes from declare_flags!, and tests/checks/metal.rs, timings.rs and the lendable-machines issue now say --record.
  • The deleted issue had one reference, in the-loader-does-only-what-must-precede-the-handover.md, and it was removed. So was the umask 077 && cat row in the host-tools table. The remaining sudo -n … Install the rule first text in the-t14-stopped-answering-ssh… is a quoted historical log, not an instruction.

No wrong-disk guard was lost. Root is now unrestricted, so I traced every root command line back to where its disk comes from:

  • Constructing an Sd. Outside tests, an Sd is built in two places only:
    • Sd::parse, which accepts Node::Scsi alone;
    • sd_disks, from the lsblk listing.
  • Jobs that name a stick. Each of Wipe, Flash, ReadLog, Create, Mount and Scratch takes an Sd value from one of three sources:
    1. located, which needs the content mark plus the recorded identity (including removable). This covers the boot path before the flash and after it.
    2. --device, which checks the identity, including removable, and skips only the content check, as the PR states.
    3. --adopt-scratch, which refuses a disk that is not listed, that carries a ToyOS layout, or that is not removable, and reads the mark back after writing.
  • --record. It runs no root command at all. It refuses a disk that is not removable before it writes the record, and it never moves an existing record.
  • The internal disk. Jobs on it use the fixed Target::internal and are gated by internal::recognise, unchanged. no_root_line_writes_the_benchs_own_partitions still pins it.
  • Other values in a root line.
    • Boot ids come only from entries_labelled, which accepts exactly four hex digits, or the dry run's 0000.
    • Retag GUIDs are typed Guid.
    • Every word goes through shell_word.
    • So dropping Mask::Hex4/Mask::Guid lost no validation that the types or the parser do not already supply.

The partition numbers taken from the image are validated three times. index is the GPT entry index + 1, from admit, which requires exactly one ESP and one TOYOS-LOG. Then:

  • flash reads start and size back from sysfs at each of those two indices and refuses any mismatch. This happens before Create uses the ESP index.
  • The boot entry is accepted only if the firmware's HD(…) names the image's ESP partition GUID. A wrong --part would be refused, and its entry deleted.
  • Mount and ReadLog use the Sd from wait_for_the_stick(Carrying::Log { index, guid }), which requires the partition at that index to carry this build's TOYOS-LOG GUID.

Layout dropping the fixed positions does not matter, because the flash rewrites the table.

r1 findings.

  • B1 (the microSD sentence): fixed. timings.rs is main's except for the one --record line.
  • B2 (Record::scratch): fixed. The record is unchanged from main, and adoption verifies Carrying::Scratch on the disk itself.
  • B3 (metal): resolved for what this PR claims.
    • The T14 boot at the head flashed, booted and passed acpi_server_death 1/1. The readback was written by the driver at toyos-orch-a73258407 and reads passed, stick_secs 0.
    • That run is also the PARTUUID-follow evidence: the post-boot located(Log) found the stick.
    • The AMD image and adoption dry runs exited 0 at the head.
  • S1 is recorded as a risk. S2, S3 and S5 are taken. S4 is taken: table() names each considered disk's partition types.

Negative controls. m1–m5 were rerun at this tree, and each failed its named test (logs in r3/). They are real mutations of located, holds and adoptable.

Host CI. It ran at a732584: 103 steps, exit 0.

Identifiers. No serial or bench partition GUID was added:

  • The GUIDs in tests are synthetic or already on main.
  • 51CC09F9-… is a type GUID drawn for this PR.
  • "SanDisk Ultra" with 60062500 sectors and 20W0003AMZ are already on main.

Findings

BLOCKER B1 — tests/metal/src/lib.rs:206 — /// The account's home on the machine is no absolute path a rule can name. is stale.

  • "a rule can name" refers to the deleted sudoers rule. Home::parse now refuses only control characters and non-normal, non-absolute paths, and the Display arm was already updated to drop the phrase.
  • Fix: delete the clause, for example /// The account's home on the machine is no normal absolute path., or delete the line.

SUGGESTION S1 — lib.rs:1030 admit versus lib.rs:531-537 Carrying::Layout — they do not hold an image to the same table.

  • Layout requires 1–2 TOYOS_BOOT partitions. admit never counts them; its test images have none.
  • An admitted image without a BOOT partition would flash, boot, and then be Unlocated on every later run until it is re-adopted with --device.
  • The doc says "as [admit] holds an image to one", which is not quite true.
  • Fix: have admit require the same 1–2 BOOT partitions, so that what is flashed is always relocatable, or drop the clause from Layout. This predates r3.

SUGGESTION S2 — evidence — amd-dry-r3.txt is a one-line summary rather than the driver's output, and the T14 r3 directory keeps the readback and the judge but not the driver's stdout (the run: sudo -n … lines and the selection line).

  • The claims are plausible and consistent with the code.
  • Publishing both stdouts, masked, to the pr-1064 evidence branch would make "selected /dev/sda by content" and "listed every sudo -n command" checkable rather than reported.

PR description (for the orchestrator).

  • "Metal: … One more T14 boot and the AMD dry runs on this head are outstanding" is stale. Both ran.
  • Under risks, "that PARTUUID follows the stick after the boot (the T14 run settles this)" is now settled by the T14 runs at 477c14c and a732584.
  • The remaining open item is sgdisk --clear after wipefs on a real adoption, which no dry run exercises.

SEND BACK

`admit` now requires one or two TOYOS-BOOT partitions beside the one or two
ROOTs, which is what `Carrying::Layout` asks of a stick. Before this, an
image with no BOOT partition would flash and boot, and every later run would
then refuse the stick as unlocated. `Roots` becomes `Slots { what, matched }`
and covers both partition kinds.

The home refusal's doc no longer mentions the deleted rule.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu

Japabu commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

r4 at af4060f: review r2 response

  • B1 fixed: the Home refusal's doc reads "no normal absolute path".
  • S1 taken: admit requires 1–2 TOYOS-BOOT partitions beside 1–2 ROOTs. Roots is now Slots { what, matched }. No selection code changed (located, Carrying and sd_disks are byte-identical to a732584). This is a refusal before the flash; the real staged acpicase image is still admitted.
  • S2 taken. The driver's own output from both T14 runs is published, masked: driver/. For the AMD, only your summary existed in the scratch tree, so that is what is published. The full dry-run output is in your message history, which I can't read; if it should be published, drop it in a file and I'll add it.
  • PR description: the metal runs are done, the PARTUUID risk is settled, and the only open item is sgdisk --clear after wipefs on a real adoption.

Checks and evidence:

  • cargo run -- --ci host: exit 0, 103 green.
  • cargo test -p toyos-metal: 82 passed.
  • m6 (the BOOT count removed) exits 101 at an_image_admits_only_a_toyos_table.
  • Evidence: r4/

@Japabu
Japabu marked this pull request as ready for review October 24, 2026 11:33
@Japabu
Japabu added this pull request to the merge queue Oct 24, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 24, 2026
@Japabu
Japabu added this pull request to the merge queue Oct 24, 2026
@Japabu
Japabu removed this pull request from the merge queue due to a manual request Oct 24, 2026
@Japabu
Japabu added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit 8348c2a Oct 27, 2026
8 checks passed
@Japabu
Japabu deleted the metal-stick-port branch October 27, 2026 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant