dissect: add --manifest switch to generate a UAPI.16 manifest file fr…

[poettering]

…om a directory tree

The UAPI.16 spec is being discussed here:

uapi-group/specifications#213

[poettering]

Do not merge until the spec is also good to be merged.

[github-actions]

Claude review of PR #41990 (97448c7)

Must fix

• Unused variable escaped — src/dissect/dissect.c:1348 — declared but never used in manifest_print_item(), will fail with -Werror
• S_ISDIR(sx->stx_mask) should be S_ISDIR(sx->stx_mode) — src/dissect/dissect.c:1386 — uses wrong struct field for file type check, causing directories to incorrectly enter the hardlink processing path
• fd leak on lseek failure in rm_rf_children — src/shared/rm-rf.c:313-315 — when REMOVE_REWIND lseek fails, returns -errno without closing the fd, violating the function's ownership contract
• Non-UTF-8 filename aborts entire manifest — src/dissect/dissect.c:1585 — a single non-UTF-8 name makes sd_json_buildo() fail with -EUCLEAN and aborts the whole recurse_dir() walk

Suggestions

• NULL from inode_type_to_string() — src/dissect/dissect.c:1397 — can return NULL for unknown inode types, producing a JSON null instead of a string
• name_to_handle_at_try_fid hard failure — src/dissect/dissect.c:1418 — non-fatal errors (e.g. -EOPNOTSUPP) abort the entire manifest, could gracefully skip hardlink detection instead
• Missing log on sd_json_buildo failure — src/dissect/dissect.c:1543 — error returned without logging, unlike the analogous call earlier in the file
• stx_size accessed without STATX_SIZE mask check — src/dissect/dissect.c:1543 — in the S_IFREG case, sx->stx_size is used after only checking STATX_TYPE, inconsistent with mtree_print_item which checks both
• --mtree-hash flag reused for manifest — src/dissect/dissect.c:1453 — the flag name and description reference mtree output only, but it now also controls manifest hashing
• get_entry() helper has no failure guard — test/units/TEST-50-DISSECT.dissect.sh:1143 — if no entry matches, produces empty output leading to confusing assertion failures
• --manifest on image only checks exit code — test/units/TEST-50-DISSECT.dissect.sh:1219 — output is discarded; consider verifying non-empty output or expected content
• type vocabulary vs UAPI.16 spec — src/dissect/dissect.c:1578 — confirm inode_type_to_string() tokens (dir/lnk/chr/blk/fifo/sock) match the spec's type enum
• Symlink target via contents/literal — src/dissect/dissect.c:1588 — confirm the spec sanctions reusing the regular-file contents/literal mechanism for symlink targets
• python3 bind unguarded under errexit — test/units/TEST-50-DISSECT.dissect.sh:1125 — a bind failure aborts the whole test even though socket coverage is best-effort

Nits

• Double blank line — src/dissect/dissect.c:1380-1382 — extra blank line between break; and case S_IFLNK:
• Misleading error message — src/dissect/dissect.c:1427 — says "Failed to symlink" but the operation is readlinkat_malloc on the hardlink database
• Pointer star style — src/dissect/dissect.c:1383 — const char* type should be const char *type
• Operator spacing — src/dissect/dissect.c:1475 — colon +1 should be colon + 1
• Hardcoded SHA256 for empty file — test/units/TEST-50-DISSECT.dissect.sh:1162 — could compute dynamically like done for regular.txt
• Hardcoded symlink size — test/units/TEST-50-DISSECT.dissect.sh:1179 — "11" is strlen of target, would be more maintainable computed
• Missing comma in man page — man/systemd-dissect.xml:272 — "Like --mtree this" should be "Like --mtree, this"
• FID error message missing path — src/dissect/dissect.c:1395 — doesn't include the file path for debugging
• Cycle avoidance startswith too broad — src/dissect/dissect.c:1525 — startswith(path, "Uapi16Manifest.") also skips non-manifest files
• Redundant db_path guard — src/dissect/dissect.c:1358 — rmdir_and_free(NULL) is a no-op, the if (hd->db_path) check is unnecessary
• Possibly dead spec URL — man/systemd-dissect.xml:267 — the UAPI.16 spec is still an unmerged proposal, the linked URL may 404
• JSON-Lines conversion assumes single-line records — test/units/TEST-50-DISSECT.dissect.sh:1146 — tr/head -n1 break if the dump ever emits multi-line records

Workflow run

[yuwata]

Making this as draft, as this has the do-not-merge tag.

[github-actions]

Claude: must-fix: The name field is built from the raw filename via SD_JSON_BUILD_STRING(path), but sd_json_variant_new_stringn() rejects any string that is not valid UTF-8 with -EUCLEAN (src/libsystemd/sd-json/sd-json.c:424). Linux filenames are arbitrary byte sequences and the image / directory tree may be untrusted, so a single non-UTF-8 (or mojibake) name anywhere in the tree makes sd_json_buildo() fail, manifest_print_item() return the error, and recurse_dir() abort — the entire --manifest run dies with "Failed to build JSON object" on the first bad name. The --mtree path tolerates such names (it prints them raw / escapes the symlink target). Consider validating or escaping the name (or skipping with a warning) so an untrusted image cannot abort manifest generation. The same applies if any intermediate path component is non-UTF-8.

[github-actions]

Claude: suggestion: The type value comes from inode_type_to_string(), which yields the systemd-internal tokens dir/lnk/chr/blk/fifo/sock (note these differ from the --mtree vocabulary dir/file/link/block/char/socket). Please confirm these exact strings match the type enum defined by the UAPI.16 Manifest Format spec (uapi-group/specifications#213) — a manifest using a different vocabulary than the spec will not validate. The CI test in the last commit hardcodes the same tokens, so it would pass even if they diverged from the spec.

[github-actions]

Claude: suggestion: For symlinks the target is emitted as contents: [{ "literal": base64(target) }]. (The base64 size argument is correct here — it is strlen(target) set in the S_IFLNK branch, not the stale UINT64_MAX default, and contents is gated on !!data so the two cases are mutually exclusive.) The open conformance question is whether the UAPI.16 spec represents a symlink target through the same contents/literal mechanism it uses for regular-file contents, or via a dedicated field. Please confirm the spec sanctions reusing contents/literal for symlink targets.

[github-actions]

Claude: nit: rmdir_and_free(NULL) is a no-op that returns NULL, so the if (hd->db_path) guard is redundant. The surrounding systemd idiom is to call such freers unconditionally: hd->db_path = rmdir_and_free(hd->db_path);

[github-actions]

Claude: nit: This links to https://uapi-group.org/specifications/specs/manifest_format/, but the UAPI.16 Manifest Format spec is still an open, unmerged proposal (uapi-group/specifications#213, and this PR carries a do-not-merge label). That URL likely does not resolve until the spec PR merges. Consider pointing at the spec PR until it is published, or holding the docs until the canonical URL exists, so the man page does not ship a dead link.

[github-actions]

Claude: suggestion: The socket coverage is gated only on command -v python3, but the bind() itself runs unguarded under errexit. If python3 is present but the bind fails for any reason (sandboxed environment, AF_UNIX restricted, an unexpected error), the whole test aborts even though the socket case is meant to be best-effort. Consider folding the bind into the guard so a failure only leaves HAVE_SOCKET=0, e.g. if command -v python3 >/dev/null 2>&1 && python3 -c '...' "$MANIFEST_ROOT/socket"; then HAVE_SOCKET=1; fi.

[github-actions]

Claude: nit: The tr -d '\036' -> JSON-Lines conversion and the head -n1 root-record extraction both rely on each manifest record being emitted as a single compact line. That holds only because the implementation dumps with no PRETTY flag; if the manifest output ever became multi-line, head -n1 would no longer capture the full root record (jq reading the whole stream would still cope, but this one root assertion would not). Selecting the root record with jq over the full stream (e.g. jq -s '.[0]' over the stripped output) would be more robust than head -n1.