feat(controller): validate rootfs + architecture on backup import/restore#31
Merged
Merged
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
5c2d1f3 to
e82ba54
Compare
e82ba54 to
329aaca
Compare
…tore The backup import accepted ANY file with no check that it is a rootfs or the correct architecture, so a 32-bit rootfs could be imported/restored into a 64-bit app (and vice-versa), or a ZIM/random file treated as a rootfs. Per the ABI-separation policy (ARM64<->ARM64, 32<->32) these must be blocked. - domain (pure, unit-tested): ElfClass (read 32/64 from an ELF header) and RootfsArchive (structural "looks like a rootfs" + pick a probe binary, prefix-tolerant for the installed-rootfs/ backup prefix). - data/RootfsArchiveValidator: lists the archive, runs the structural check, and probes one internal binary's ELF class against the app's ABI (Process.is64Bit()). Hard-blocks NOT_A_ROOTFS and a DEFINITE WRONG_ARCH; if the arch can't be determined it does not block on arch (no false positives). - Two gates: import (DeployFragment.importBackupSafely rejects + deletes the copied file) and restore (TarExtractor gains a validateRootfs overload that reuses its existing D11 entry listing; the 4-arg call stays validateRootfs=false so the rootfs-download install path is unchanged). New strings (en + es). Pending (documented, not here): an internal rootfs manifest emitted by the rootfs builder + in-app backup creation (authoritative "is it our rootfs", validated soft->strict), and the arbitrary-file attack-vector analysis. Needs on-device verification with a real 32-bit and 64-bit backup so the probe heuristic does not false-positive on legitimate restores.
…ft + alert) Builds on the import/restore validation: now reads the build's identity manifest `installed-rootfs/iiab/.iiab-rootfs.json` (canonical contract: docs/ROOTFS_MANIFEST.md), which the rootfs builder (PR #24) emits as the first tar member. - New deploy/data/RootfsManifest: a dependency-free reader that parses only the first few 512-byte tar headers (the identity member is packed first) to get {kind, arch} — no reliance on libtar `--occurrence`, fast (a few KB). - RootfsArchiveValidator: when the manifest is present it authoritatively gates `kind` (must be "iiab-rootfs") and `arch` (must equal this app's ABI); the ELF probe is skipped. When absent it returns OK_NO_MANIFEST and falls back to the existing ELF/structure heuristic. - Soft phase (this version): a missing manifest does NOT block — import shows a non-blocking "manifest not found" warning and proceeds; a later version will validate silently / then strictly. Wrong kind/arch is still hard-blocked. - New string install_warn_manifest_missing (en + es). Next (separate PR): the integrity check (iiab-tree-sha256-v1 -> Result.CORRUPT, a Java ustar/pax reader mirroring tools/iiab_tree_hash.py) and the in-app backup-writer emitting both members.
329aaca to
abcd769
Compare
luisguzman-adfa
added a commit
that referenced
this pull request
Jun 24, 2026
- Status banner: Phase 0 done + Phase 1 core-complete; Phase 2 not started (~37 raw threads); Phase 3 partial (slices land but god classes grew — DeployFragment ~3022, MainActivity ~2384); M15 done, M8/M9 open. - Phase 1 line: F15 A+B (#30/#32) + rootfs validation (#31) + integrity (#37) done. - Integrity/writer entry: marked DONE via #37 (was 'next PR'). - F4 + M7: confirmed mitigated by inspection (onPause teardown + remove-before-post). - Lint workaround: added root-cause analysis with concrete carve targets (addNewTerminalSession ~727 LOC; DeployFragment bind* methods) for D1/F1. Docs only; no code change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The backup import accepted any file with no check that it is a rootfs or the correct architecture, so a 32-bit rootfs could be imported/restored into a 64-bit app (and vice-versa), or a ZIM / random file treated as a rootfs. Per the ABI-separation policy (ARM64↔ARM64, 32↔32) these must be blocked, at both the import gate and the restore gate.
Changes (layered
org.iiab.controller.deploy)domain/(pure JVM, unit-tested):ElfClass(read 32/64-bit from an ELF header) andRootfsArchive(structural "looks like a rootfs" + pick a probe binary; prefix-tolerant for theinstalled-rootfs/backup prefix).data/RootfsArchiveValidator: lists the archive, runs the structural check, and probes one internal binary's ELF class against the app's ABI (Process.is64Bit()). Hard-blocksNOT_A_ROOTFSand a definiteWRONG_ARCH; if the arch can't be determined it does not block on arch (avoids false positives), while the structural check still applies.DeployFragment.importBackupSafelyrejects + deletes the copied file) and restore (TarExtractorgains avalidateRootfsoverload reusing its existing D11 listing; the 4-arg call staysvalidateRootfs=false, so the rootfs-download install path is unchanged). New strings (en + es).Design notes
Hard-block on a positively-detected wrong architecture; "undetermined" ≠ "wrong", so a legitimate restore is never false-blocked. Honors the ABI policy (enforcement on the import/restore side); the existing 32-on-64 infra is not removed.
Pending (documented, NOT in this PR)
Testing
ElfClassTest+RootfsArchiveTest(pure JVM, CI gate). Thetar-probing data layer is Android-bound — needs on-device verification with a real 32-bit and a real 64-bit backup (legitimate restores still pass; wrong-arch is blocked).