The partition probe fails closed on every path (R-1093 review, unreleased)
PART_ENTRY_* comes from the parent's table, so it proves nothing about a partition's contents unless blkid also completed: lsblk's PTTYPE is now set aside only on a blkid exit 0 (exit 2, an I/O error or a missing binary keep "device has a partition table"). An lsblk that exits 0 with output that does not parse into a device, or lists none, leaves the device unprobed instead of blank (a hole that predates R-1093). Six more TestInspect_R1093_PartitionVerdicts rows, each red-proved.
This commit is contained in:
1 parent
8561dc8a5c
commit
0963bae3c3
3 files changed
+60
-20
No files matched your search
@@ -11,6 +11,12 @@ Committed, not released — the release and its vouch are the operator's.
|
|||||||
that differs keeps the old, safe reading. **No new command, no sudoers or config-bundle change** (the two probes are
|
that differs keeps the old, safe reading. **No new command, no sudoers or config-bundle change** (the two probes are
|
||||||
the same allow-listed argv). Test `TestInspect_R1093_PartitionVerdicts` (8 rows), red-proved: with the old
|
the same allow-listed argv). Test `TestInspect_R1093_PartitionVerdicts` (8 rows), red-proved: with the old
|
||||||
`if pttype != ""` both empty-partition rows fail `DataBearing=true reason="device has a partition table"`.
|
`if pttype != ""` both empty-partition rows fail `DataBearing=true reason="device has a partition table"`.
|
||||||
|
**Hardened after the same night's review (fail-closed on every path):** `PART_ENTRY_*` is read from the PARENT's
|
||||||
|
table, so lsblk's PTTYPE is set aside only when blkid also **exited 0** — exit 2, an I/O error or a missing binary
|
||||||
|
keep „device has a partition table"; and an `lsblk` that exits 0 with output that does not parse into a device (or
|
||||||
|
lists none) now leaves the device UNPROBED („could not be reliably inspected") instead of „blank" — that hole
|
||||||
|
predates R-1093. Six more rows in the same test, each red-proved (`blkidErr` check removed → „device is blank";
|
||||||
|
`&& parsed` removed → `Probed=true … „device is blank"`).
|
||||||
- **R-978 — an abandoned second escrow ceremony no longer makes the customer's recovery code useless.** The root
|
- **R-978 — an abandoned second escrow ceremony no longer makes the customer's recovery code useless.** The root
|
||||||
ceremony (the FELHOM_ESCROW argv, unchanged: `--upload --output=json`) used to upload the new package before the new
|
ceremony (the FELHOM_ESCROW argv, unchanged: `--upload --output=json`) used to upload the new package before the new
|
||||||
code was claimed, and the hub overwrites the current package when the sealed secrets are unchanged — so a customer
|
code was claimed, and the hub overwrites the current package when the sealed secrets are unchanged — so a customer
|
||||||
|
|||||||
+18
-16
@@ -419,7 +419,7 @@ func (h *SudoHostOps) InspectDevice(ctx context.Context, device string) (DeviceP
|
|||||||
// any TYPE/PTTYPE/USAGE line is positive data-bearing evidence. Its exit code is NOT relied
|
// any TYPE/PTTYPE/USAGE line is positive data-bearing evidence. Its exit code is NOT relied
|
||||||
// on (blkid exits 2 on a blank device) — output presence is what matters. A broken/empty
|
// on (blkid exits 2 on a blank device) — output presence is what matters. A broken/empty
|
||||||
// blkid simply adds no positive evidence; lsblk (below) is the read-success authority.
|
// blkid simply adds no positive evidence; lsblk (below) is the read-success authority.
|
||||||
bout, _, _ := h.runner.Run(ctx, h.bins.Blkid, "-p", "-o", "export", device)
|
bout, _, berr := h.runner.Run(ctx, h.bins.Blkid, "-p", "-o", "export", device)
|
||||||
blk := parseBlkidExport(bout)
|
blk := parseBlkidExport(bout)
|
||||||
for k, v := range blk {
|
for k, v := range blk {
|
||||||
switch k {
|
switch k {
|
||||||
@@ -440,11 +440,11 @@ func (h *SudoHostOps) InspectDevice(ctx context.Context, device string) (DeviceP
|
|||||||
// lsblk -J is the READ-SUCCESS authority + the partition/mount view. It exits 0 on any valid
|
// lsblk -J is the READ-SUCCESS authority + the partition/mount view. It exits 0 on any valid
|
||||||
// device (blank or not), so a clean parse means the agent reliably read the device. If lsblk
|
// device (blank or not), so a clean parse means the agent reliably read the device. If lsblk
|
||||||
// fails, Probed stays false → DataBearing()=true (fail-safe — never call a device blank on a
|
// fails, Probed stays false → DataBearing()=true (fail-safe — never call a device blank on a
|
||||||
// failed read).
|
// failed read). An exit 0 whose output does not parse into a device is NOT a read either (R-1093
|
||||||
|
// review): it stays unprobed, never "blank".
|
||||||
lout, _, lerr := h.runner.Run(ctx, h.bins.Lsblk, "-J", "-o", "NAME,FSTYPE,PTTYPE,MOUNTPOINT", device)
|
lout, _, lerr := h.runner.Run(ctx, h.bins.Lsblk, "-J", "-o", "NAME,FSTYPE,PTTYPE,MOUNTPOINT", device)
|
||||||
if lerr == nil {
|
if hasChildren, mounted, fstype, pttype, parsed := parseLsblkDevice(lout); lerr == nil && parsed {
|
||||||
probe.Probed = true
|
probe.Probed = true
|
||||||
hasChildren, mounted, fstype, pttype := parseLsblkDevice(lout)
|
|
||||||
if hasChildren {
|
if hasChildren {
|
||||||
probe.HasPartitions = true
|
probe.HasPartitions = true
|
||||||
}
|
}
|
||||||
@@ -457,7 +457,7 @@ func (h *SudoHostOps) InspectDevice(ctx context.Context, device string) (DeviceP
|
|||||||
probe.FSType = fstype
|
probe.FSType = fstype
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if pttype != "" && !inheritedPartitionTable(blk, pttype) {
|
if pttype != "" && !inheritedPartitionTable(blk, berr, pttype) {
|
||||||
probe.HasPartitionTable = true
|
probe.HasPartitionTable = true
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -470,13 +470,14 @@ func (h *SudoHostOps) InspectDevice(ctx context.Context, device string) (DeviceP
|
|||||||
//
|
//
|
||||||
// The partition's OWN signatures come from `blkid -p` on the partition itself (above): a nested table
|
// The partition's OWN signatures come from `blkid -p` on the partition itself (above): a nested table
|
||||||
// inside it is its PTTYPE, a filesystem is its TYPE. So lsblk's value is set aside ONLY when blkid
|
// inside it is its PTTYPE, a filesystem is its TYPE. So lsblk's value is set aside ONLY when blkid
|
||||||
// positively proved two things — that it read the device (it printed output), and that the device is a
|
// positively proved two things — that it completed its probe of the device (exit 0: PART_ENTRY_* alone
|
||||||
// partition of a table of exactly that type (PART_ENTRY_NUMBER + PART_ENTRY_SCHEME == lsblk's PTTYPE).
|
// comes from the PARENT's table, so output without a clean exit does not prove the partition's contents
|
||||||
// Anything else keeps the old, safe reading: a blkid that failed or printed nothing, a whole disk, or a
|
// were read), and that the device is a partition of a table of exactly that type (PART_ENTRY_NUMBER +
|
||||||
// scheme that differs from lsblk's (then lsblk sees a table blkid did not attribute to the parent).
|
// PART_ENTRY_SCHEME == lsblk's PTTYPE). Anything else keeps the old, safe reading: a blkid that failed,
|
||||||
// Pinned by TestInspect_R1093_* (hostops_disk_test.go).
|
// exited non-zero or printed nothing, a whole disk, or a scheme that differs from lsblk's (then lsblk sees
|
||||||
func inheritedPartitionTable(blkid map[string]string, lsblkPTType string) bool {
|
// a table blkid did not attribute to the parent). Pinned by TestInspect_R1093_* (hostops_disk_test.go).
|
||||||
if blkid["PART_ENTRY_NUMBER"] == "" {
|
func inheritedPartitionTable(blkid map[string]string, blkidErr error, lsblkPTType string) bool {
|
||||||
|
if blkidErr != nil || blkid["PART_ENTRY_NUMBER"] == "" {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
scheme := blkid["PART_ENTRY_SCHEME"]
|
scheme := blkid["PART_ENTRY_SCHEME"]
|
||||||
@@ -561,13 +562,14 @@ type lsblkDevice struct {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// parseLsblkDevice parses `lsblk -J -o NAME,FSTYPE,PTTYPE,MOUNTPOINT <device>` for the top device:
|
// parseLsblkDevice parses `lsblk -J -o NAME,FSTYPE,PTTYPE,MOUNTPOINT <device>` for the top device:
|
||||||
// whether it has child partitions, is mounted (itself or any child), and its fstype/pttype.
|
// whether it has child partitions, is mounted (itself or any child), and its fstype/pttype. parsed=false
|
||||||
func parseLsblkDevice(out []byte) (hasChildren, mounted bool, fstype, pttype string) {
|
// when the output is not a device listing — the caller must then treat the device as unprobed.
|
||||||
|
func parseLsblkDevice(out []byte) (hasChildren, mounted bool, fstype, pttype string, parsed bool) {
|
||||||
var doc struct {
|
var doc struct {
|
||||||
BlockDevices []lsblkDevice `json:"blockdevices"`
|
BlockDevices []lsblkDevice `json:"blockdevices"`
|
||||||
}
|
}
|
||||||
if json.Unmarshal(out, &doc) != nil || len(doc.BlockDevices) == 0 {
|
if json.Unmarshal(out, &doc) != nil || len(doc.BlockDevices) == 0 {
|
||||||
return false, false, "", ""
|
return false, false, "", "", false
|
||||||
}
|
}
|
||||||
d := doc.BlockDevices[0]
|
d := doc.BlockDevices[0]
|
||||||
fstype, pttype = d.FSType, d.PTType
|
fstype, pttype = d.FSType, d.PTType
|
||||||
@@ -578,7 +580,7 @@ func parseLsblkDevice(out []byte) (hasChildren, mounted bool, fstype, pttype str
|
|||||||
mounted = true
|
mounted = true
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return hasChildren, mounted, fstype, pttype
|
return hasChildren, mounted, fstype, pttype, true
|
||||||
}
|
}
|
||||||
|
|
||||||
// NoopHostOps is the safe fallback when the privileged surface is unavailable or declined
|
// NoopHostOps is the safe fallback when the privileged surface is unavailable or declined
|
||||||
|
|||||||
@@ -238,9 +238,12 @@ const lsblkPartInheritsGPT = `{"blockdevices":[{"name":"sdb1","fstype":null,"ptt
|
|||||||
// TestInspect_R1093_PartitionVerdicts pins the CONSEQUENCE (DataBearing + Reason, what decides whether
|
// TestInspect_R1093_PartitionVerdicts pins the CONSEQUENCE (DataBearing + Reason, what decides whether
|
||||||
// the typed erase confirmation is asked) for a partition, in both directions.
|
// the typed erase confirmation is asked) for a partition, in both directions.
|
||||||
//
|
//
|
||||||
// COMPANION RED-PROOF (observed 2026-10-18): restore the pre-fix `if pttype != "" {` (drop the
|
// COMPANION RED-PROOFS (observed 2026-10-18): (1) restore the pre-fix `if pttype != "" {` (drop the
|
||||||
// inheritedPartitionTable call) → BOTH empty-partition rows (GPT and dos) FAIL with
|
// inheritedPartitionTable call) → BOTH empty-partition rows (GPT and dos) FAIL with
|
||||||
// `DataBearing=true reason="device has a partition table", want blank`. Restored.
|
// `DataBearing=true reason="device has a partition table", want blank`. (2) drop the `blkidErr != nil`
|
||||||
|
// check → the "exit 2" and "I/O error" rows FAIL with `DataBearing=false reason="device is blank"`.
|
||||||
|
// (3) drop `&& parsed` → both lsblk-exit-0 rows FAIL with `Probed=true … reason="device is blank"`
|
||||||
|
// (the unparsable-output hole predates R-1093; closed here). Restored.
|
||||||
func TestInspect_R1093_PartitionVerdicts(t *testing.T) {
|
func TestInspect_R1093_PartitionVerdicts(t *testing.T) {
|
||||||
cases := []struct {
|
cases := []struct {
|
||||||
name string
|
name string
|
||||||
@@ -248,11 +251,13 @@ func TestInspect_R1093_PartitionVerdicts(t *testing.T) {
|
|||||||
blkid string
|
blkid string
|
||||||
blkidErr error
|
blkidErr error
|
||||||
lsblk string
|
lsblk string
|
||||||
|
lsblkErr error
|
||||||
wantBear bool
|
wantBear bool
|
||||||
wantReason string
|
wantReason string
|
||||||
|
unprobed bool // the verdict must be "could not be reliably inspected"
|
||||||
}{
|
}{
|
||||||
{name: "empty GPT partition is blank (the R-1093 case)", dev: "/dev/sdb1",
|
{name: "empty GPT partition is blank (the R-1093 case)", dev: "/dev/sdb1",
|
||||||
blkid: blkidEmptyGPTPart, blkidErr: errors.New("exit status 2"), lsblk: lsblkPartInheritsGPT,
|
blkid: blkidEmptyGPTPart, lsblk: lsblkPartInheritsGPT,
|
||||||
wantBear: false, wantReason: "device is blank"},
|
wantBear: false, wantReason: "device is blank"},
|
||||||
{name: "empty MBR (dos) partition is blank", dev: "/dev/sdb2",
|
{name: "empty MBR (dos) partition is blank", dev: "/dev/sdb2",
|
||||||
blkid: "DEVNAME=/dev/sdb2\nPART_ENTRY_SCHEME=dos\nPART_ENTRY_TYPE=0x83\nPART_ENTRY_NUMBER=2\nPART_ENTRY_DISK=8:16\n",
|
blkid: "DEVNAME=/dev/sdb2\nPART_ENTRY_SCHEME=dos\nPART_ENTRY_TYPE=0x83\nPART_ENTRY_NUMBER=2\nPART_ENTRY_DISK=8:16\n",
|
||||||
@@ -269,6 +274,26 @@ func TestInspect_R1093_PartitionVerdicts(t *testing.T) {
|
|||||||
{name: "blkid printed nothing: lsblk's PTTYPE stands (fail-safe)", dev: "/dev/sdb1",
|
{name: "blkid printed nothing: lsblk's PTTYPE stands (fail-safe)", dev: "/dev/sdb1",
|
||||||
blkid: "", blkidErr: errors.New("sudo: a password is required"), lsblk: lsblkPartInheritsGPT,
|
blkid: "", blkidErr: errors.New("sudo: a password is required"), lsblk: lsblkPartInheritsGPT,
|
||||||
wantBear: true, wantReason: "device has a partition table"},
|
wantBear: true, wantReason: "device has a partition table"},
|
||||||
|
// Fail-closed paths (the 2026-10-18 review): PART_ENTRY_* is read from the PARENT's table, so it
|
||||||
|
// proves nothing about the partition's contents unless blkid also completed (exit 0).
|
||||||
|
{name: "blkid exit 2 with only PART_ENTRY output: lsblk stands", dev: "/dev/sdb1",
|
||||||
|
blkid: blkidEmptyGPTPart, blkidErr: errors.New("exit status 2"), lsblk: lsblkPartInheritsGPT,
|
||||||
|
wantBear: true, wantReason: "device has a partition table"},
|
||||||
|
{name: "blkid I/O error after PART_ENTRY output: lsblk stands", dev: "/dev/sdb1",
|
||||||
|
blkid: blkidEmptyGPTPart, blkidErr: errors.New("exit status 1"), lsblk: lsblkPartInheritsGPT,
|
||||||
|
wantBear: true, wantReason: "device has a partition table"},
|
||||||
|
{name: "blkid binary missing: lsblk stands", dev: "/dev/sdb1",
|
||||||
|
blkid: "", blkidErr: errors.New(`exec: "/usr/sbin/blkid": file does not exist`), lsblk: lsblkPartInheritsGPT,
|
||||||
|
wantBear: true, wantReason: "device has a partition table"},
|
||||||
|
{name: "lsblk exit 0 with unparsable output: unprobed", dev: "/dev/sdb1",
|
||||||
|
blkid: blkidEmptyGPTPart, lsblk: `lsblk: something odd`,
|
||||||
|
wantBear: true, unprobed: true},
|
||||||
|
{name: "lsblk exit 0 with no device listed: unprobed", dev: "/dev/sdb1",
|
||||||
|
blkid: blkidEmptyGPTPart, lsblk: `{"blockdevices":[]}`,
|
||||||
|
wantBear: true, unprobed: true},
|
||||||
|
{name: "lsblk failed: unprobed", dev: "/dev/sdb1",
|
||||||
|
blkid: blkidEmptyGPTPart, lsblk: "", lsblkErr: errors.New("exit status 32"),
|
||||||
|
wantBear: true, unprobed: true},
|
||||||
{name: "scheme differs from lsblk's PTTYPE: lsblk stands", dev: "/dev/sdb1",
|
{name: "scheme differs from lsblk's PTTYPE: lsblk stands", dev: "/dev/sdb1",
|
||||||
blkid: strings.Replace(blkidEmptyGPTPart, "PART_ENTRY_SCHEME=gpt", "PART_ENTRY_SCHEME=dos", 1),
|
blkid: strings.Replace(blkidEmptyGPTPart, "PART_ENTRY_SCHEME=gpt", "PART_ENTRY_SCHEME=dos", 1),
|
||||||
lsblk: lsblkPartInheritsGPT,
|
lsblk: lsblkPartInheritsGPT,
|
||||||
@@ -285,12 +310,19 @@ func TestInspect_R1093_PartitionVerdicts(t *testing.T) {
|
|||||||
t.Run(c.name, func(t *testing.T) {
|
t.Run(c.name, func(t *testing.T) {
|
||||||
r := &scriptedRunner{
|
r := &scriptedRunner{
|
||||||
outputs: map[string][]byte{"blkid": []byte(c.blkid), "lsblk": []byte(c.lsblk)},
|
outputs: map[string][]byte{"blkid": []byte(c.blkid), "lsblk": []byte(c.lsblk)},
|
||||||
errs: map[string]error{"blkid": c.blkidErr},
|
errs: map[string]error{"blkid": c.blkidErr, "lsblk": c.lsblkErr},
|
||||||
}
|
}
|
||||||
p, err := newSudo(r).InspectDevice(context.Background(), c.dev)
|
p, err := newSudo(r).InspectDevice(context.Background(), c.dev)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
|
if c.unprobed {
|
||||||
|
if p.Probed || !p.DataBearing() || p.Reason() != "device could not be reliably inspected" {
|
||||||
|
t.Fatalf("Probed=%v DataBearing=%v reason=%q, want unprobed + data-bearing (probe %+v)",
|
||||||
|
p.Probed, p.DataBearing(), p.Reason(), p)
|
||||||
|
}
|
||||||
|
return
|
||||||
|
}
|
||||||
if !p.Probed {
|
if !p.Probed {
|
||||||
t.Fatal("expected a clean probe (lsblk read cleanly)")
|
t.Fatal("expected a clean probe (lsblk read cleanly)")
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user