fsck: Add read-only diagnostic report for composefs state by cgwalters-bot · Pull Request #2497 · bootc-dev/bootc

cgwalters-bot · 2026-09-24T17:35:03Z

Why

Issue #2422 shows that missing composefs OCI metadata can make status, upgrade, and switch fail even after booting an older deployment. A read-only report makes it possible to collect boot, deployment, repository, and image evidence without mutating a broken system or stopping at the first missing input.

The collector bounds its reads. Fatal errors (no sysroot) fail the command; everything else is recorded as a typed finding and collection continues. This is a large patch; please review the report schema and overall approach before treating it as ready to merge.

The content of Colin's commit (fsck: Add read-only diagnostic report) changed in the review rework, which also took in Johan's comments, so his Signed-off-by was dropped pending his re-review.

Verification

On a 16-core RHEL 10 devspace, at adda0ae (same tree as f074a15):

cargo test -p bootc-lib: 293 passed
cargo clippy -p bootc-lib: no findings in fsck.rs under CI's lint set
just test-tmt-baseconfig var-volatile (composefs, readonly plan incl. 015-test-fsck: 0 findings): passed
plan-24-image-upgrade-reboot (composefs, grub/ext4/bls): passed

Related: #2422

Assisted-by: AI

Generated-by: https://github.com/cgwalters/#llms

cgwalters · 2026-09-24T18:51:37Z

+    collect_readonly_with_roots(&roots)
+}
+
+fn collect_readonly_with_roots(roots: &ReadonlyRoots<'_>) -> ReadonlyReport {


This is way too big of a function, needs to be split up

Split up: collect_readonly_with_roots now just sequences composefs_report, collect_deployments/collect_deployment, resolve_manifest_stream and collect_host. The ReadonlyRoots struct is gone; four of its six dirs were never used. All of this is squashed into your commit, now a030d83 (fsck: Add read-only diagnostic report), with your sign-off kept.

cgwalters · 2026-09-24T18:52:43Z

+        detail: sanitize(detail),
+    }
+}
+fn read_composefs_cmdline(


Shouldn't reimplement this

Now uses ComposefsCmdline::find_in_cmdline. The old parser only accepted 64 hex digits, so it redacted every real (sha512) composefs= value.

cgwalters · 2026-09-24T18:54:54Z

+    truncated: &mut bool,
+    total: &mut u64,
+) -> RuntimeStaged {
+    let Ok(dir) = open_dir_beneath(sysroot, Path::new("run/composefs")) else {


While fsck needs to run to completion, we also should not just completely swallow errors.

This can also use cap-std-ext open_dir_optional.

I think our fsck infrastructure should be like the linting code - we have a Result<Result<>> or really Result<Vec<Result<>> or so, the outer is really truly fatal/unexpected runtime errors, the inner Vec<Result> is any errors we encoutered while running fsck.

Opens now use open_dir_optional: a missing dir is None, and any other error becomes a finding (open_dir_or_record). That covers the staged marker, the deploy dir, the repository and the BLS/GRUB dirs. The report now has the two levels you described: collect_readonly_host returns anyhow::Result for fatal errors (e.g. no sysroot), and everything else goes into the findings list while collection continues. I left the existing FsckResult/FSCK_CHECKS alone; whether to convert those too is asked in cgwalters-forge/tracker#161.

cgwalters · 2026-09-24T18:55:05Z

+        Err(e) => {
+            findings.push(finding(
+                "STAGED_MARKER_UNREADABLE",
+                "run/composefs/staged-deployment",


Reuse existing const

Now uses STATE_DIR_RELATIVE, COMPOSEFS, COMPOSEFS_TRANSIENT_STATE_DIR, COMPOSEFS_STAGED_DEPLOYMENT_FNAME and ORIGIN_KEY_IMAGE/ORIGIN_KEY_MANIFEST_DIGEST.

cgwalters · 2026-09-24T18:55:30Z

+    Ok(dir)
+}
+
+fn open_from(dir: &Dir, relative: &Path) -> std::io::Result<File> {


Don't reimplement stuff like this use cap-std or have a strong rationale why it's not needed

Dropped open_from/open_beneath/open_dir_from/metadata_beneath and the raw openat walks in favor of cap-std, which already confines every lookup to the dir. The only helper left is open_nofollow, which checks symlink_metadata first so that a symlinked final component is reported instead of being read.

cgwalters · 2026-09-24T18:56:39Z

+        Ok(dir) => dir,
+        Err(e) => {
+            findings.push(finding(
+                "REPOSITORY_DIRECTORY_UNREADABLE",


The SCREAMING_SNAKE_CASE is ugly, let's use PascalCase basically each of these is like an error enum per above

Codes are now a FindingCode enum that serializes as PascalCase (e.g. DeploymentOriginMissing). is_collection_error() replaces the string suffix matching.

cgwalters · 2026-09-24T18:57:03Z

+        })
+        .collect()
+}
+impl ImageReport {


Btw doesn't cargo fmt want a space here? We should ensure it does...

rustfmt doesn't add blank lines between items. Its only knob for that, blank_lines_lower_bound, is nightly-only and applies inside blocks too, so we can't enforce it there. I added the blank lines across fsck.rs by hand.

cgwalters · 2026-09-24T18:57:28Z

+    }
+    edges
+}
+fn valid_sha256(v: &str) -> bool {


There's sstuff for this in oci-spec, avoid reimplmeenting your own validators

Now parses with oci-spec's Digest::from_str, which accepts any valid algorithm.

Johan-Liebert1

Not a full review because Github is unable to load a ~3k diff...

Johan-Liebert1 · 2026-09-25T08:39:44Z

+const MAX_ARTIFACTS: usize = 256;
+const MAX_LAYERS: usize = 256;
+const MAX_REPORT_STRING: usize = 256;
+const EFI_LOADER_INFO: &str = "LoaderInfo-4a67b082-0a4c-41cf-b6c7-440b29bb8c4f";


We have this constant elsewhere

Fixed: it now uses crate::install::EFI_LOADER_INFO.

Johan-Liebert1 · 2026-09-25T08:43:42Z

+                repo: open_dir_from(&sysroot, Path::new("composefs")).ok(),
+                deploy: open_dir_from(&sysroot, Path::new("state/deploy")).ok(),
+                run: open_dir_from(&sysroot, Path::new("run")).ok(),
+                boot: open_dir_from(&sysroot, Path::new("boot")).ok(),


For grub classic systems we have entries in /sysroot/boot

Those are covered: the BLS roots are relative to the sysroot, so boot/loader/entries (and .staged) is /sysroot/boot/loader/entries, and GRUB user.cfg is read from boot/grub2 and boot/grub. The ReadonlyRoots.boot field this was on was never used and has been removed.

Johan-Liebert1 · 2026-09-25T08:45:07Z

+                deploy: open_dir_from(&sysroot, Path::new("state/deploy")).ok(),
+                run: open_dir_from(&sysroot, Path::new("run")).ok(),
+                boot: open_dir_from(&sysroot, Path::new("boot")).ok(),
+                esp: open_dir_from(&sysroot, Path::new("boot/efi")).ok(),


If the image has /efi with XBOOTLDR then ESP is mounted at /efi

Fixed: efi/loader/entries (and .staged) are now BLS roots too, and boot artifacts are also looked up under efi/. The ESP mount check already accepted /sysroot/efi.

Johan-Liebert1 · 2026-09-25T08:46:19Z

+    let mut findings = Vec::new();
+    let mut truncated = false;
+    let mut read_bytes = 0;
+    let composefs_dir = open_dir_beneath(sysroot, Path::new("composefs"));


Don't we already have this in ReadonlyRoots.repo?

Yes. ReadonlyRoots is gone and the repository is opened once in composefs_report.

Johan-Liebert1 · 2026-09-25T08:50:18Z

+        .and_then(|dir| metadata_at(dir, "meta.json".as_ref()).ok());
+    let meta_contents = meta_fs
+        .as_ref()
+        .is_some_and(|m| m.file_type == EntryType::File)


If it's a symlink (which we're not treating as an error) we won't read it.

Right. A symlinked composefs/meta.json now gets a RepositoryMetadataSymlink finding. A symlink there, or any error other than not-found, used to look the same as the file being absent.

Johan-Liebert1 · 2026-09-25T09:00:16Z

+                Ok(ini) => {
+                    deployment.image = ini
+                        .get::<String>("origin", "container")
+                        .map(|v| redact_ref(&v));


Wouldn't redacting be very unhelpful here as we won't be able to determine if the origin points to the correct oci image or not?

Agreed. The origin image ref is now reported as-is, digest-pinned @sha256: refs included. Only URL-style user:password@ credentials are redacted.

Johan-Liebert1 · 2026-09-25T09:05:22Z

+    truncated: &mut bool,
+    total: &mut u64,
+) -> RuntimeStaged {
+    let Ok(dir) = open_dir_beneath(sysroot, Path::new("run/composefs")) else {


this is incorrect. run/composefs is relative to / and not /sysroot

Good catch, fixed: the staged marker is now read from the host root's run/composefs, and there's a unit test with separate host and sysroot dirs.

When the head is his own commit, he acted on his review himself, as on bootc-dev/bootc#2497, where he force-pushed after his line comments and the PR was then listed as outstanding every sweep. Pushes by anyone else, like GitHub's "Update branch" (web-flow), still answer nothing. Generated-by: AI

cgwalters-bot · 2026-09-26T04:07:23Z

Rebased onto main; 1 commit, no content change.

Generated-by: https://github.com/cgwalters/#llms

cgwalters-bot · 2026-09-26T06:05:09Z

Fixed the CI failures, squashed into the one commit (now 94f6532). The failing baseconfigs, test-upgrade and sealed UKI legs all failed in 015-test-fsck.nu on false findings, not on the transient/volatile layouts:

OCI_MANIFEST_DIGEST_INVALID: manifests and configs are stored as splitstreams, so the report now reads the JSON through SplitStreamReader instead of hashing the stream file.
BOOT_ARTIFACT_MISSING: GRUB BLS paths written as /boot/... (no separate /boot partition) are now also resolved against the physical root.
The test asserted describe starts with list, but nushell reports a non-empty list of records as table<...>.

Tested on a 16-core devspace: cargo test -p bootc-lib --lib fsck (19 passed), just test-tmt-baseconfig var-volatile and just test-upgrade (composefs, grub, ext4, centos-10), both passing plan-01-readonly. Your review comments aren't addressed yet.

Generated-by: https://github.com/cgwalters/#llms

Provide the diagnostic evidence requested by bootc-dev#2422 without changing the booted system. Collection is capability-confined and non-mutating, includes composefs and boot evidence, and reports bounded partial failures rather than aborting on the first unavailable input. Assisted-by: AI Related: bootc-dev#2422

bootc-bot Bot requested a review from cgwalters September 24, 2026 17:35

github-actions Bot added the area/documentation Updates to the documentation label Sep 24, 2026

cgwalters reviewed Sep 24, 2026

View reviewed changes

cgwalters-bot added this to Workstream Sep 24, 2026

cgwalters-bot moved this to In Review in Workstream Sep 24, 2026

cgwalters force-pushed the issue-2422-fsck-report branch from 411768d to 95399e3 Compare September 24, 2026 21:55

Johan-Liebert1 reviewed Sep 25, 2026

View reviewed changes

cgwalters-bot added this to Composefs Stable Sep 25, 2026

cgwalters-bot force-pushed the issue-2422-fsck-report branch 2 times, most recently from f06e364 to 94f6532 Compare September 26, 2026 06:05

cgwalters-bot force-pushed the issue-2422-fsck-report branch 2 times, most recently from a030d83 to adda0ae Compare September 26, 2026 22:31

cgwalters-bot force-pushed the issue-2422-fsck-report branch from adda0ae to f074a15 Compare September 26, 2026 22:32

Conversation

cgwalters-bot commented Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

Verification

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cgwalters-bot Sep 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Johan-Liebert1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cgwalters-bot commented Sep 26, 2026

Uh oh!

cgwalters-bot commented Sep 26, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

cgwalters-bot commented Sep 24, 2026 •

edited

Loading

cgwalters-bot Sep 26, 2026 •

edited

Loading