fix(guest-image): allowlist layer entry types and fixture-test the unpack (RIG-3850) - #1334
Open
rigel-mintaka wants to merge 5 commits into
Open
rigel-mintaka wants to merge 5 commits into
rigel-mintaka wants to merge 5 commits into
Conversation
…pack (RIG-3850) Closes two of the three deferred findings on guest rootfs assembly. - **Header allowlist.** Before extracting each OCI layer, `assemble-layers.sh` rejects any entry that is not a regular file, directory, symlink or hardlink. It also rejects a hardlink that is absolute or has a `..` component, a symlink to an absolute path outside `/nix/store`, and a relative symlink that climbs above the archive root. Across all 120 pinned layers the real image ships 18,367 files, 4,241 directories, 2,954 symlinks (790 absolute, all under `/nix/store`), no escaping relative symlinks, and no hardlinks, so the rule takes nothing away. - **Shared script.** The per-layer unpack, the directory-mode restore and the userland-contract resolver move out of the derivation's inline shell into `guest-image/assemble-layers.sh`, so the rootfs build and its tests run the same code. The published rootfs is byte-identical before and after: `sha256 eca6ee5ec8b17ae5…` on both builds. - **Fixtures.** `compass-guest-assembly-tests` builds tiny deterministic layers in the sandbox. It covers whiteout-then-re-add, an opaque directory with same-layer siblings, a layer replacing a symlink parent with a directory, the contract resolving through an in-image `/nix/store` link, a missing contract binary, a FIFO, an escaping symlink, a host-path symlink and an escaping hardlink. Each guard was red-checked: disabling it made its fixture fail. It runs as `guest-image:test`, and `ci` depends on it. The redirect finding stays open. curl's `--location` drops the `Authorization` header on the cross-host 307 to `pkg-containers.githubusercontent.com` (verified with `curl -v`), and the bearer is GHCR's anonymous constant, so there is no credential to leak. Spec-impact: none. Refs RIG-3850 Co-authored-by: Matt Wilkinson <matt@rigel.build>
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
Compass engineering docs preview: https://compass-managed-rig-3850-lay.compass-eng-docs.pages.dev Deployed from |
…cape (RIG-3850) Review findings on the layer allowlist: - A member name containing " link to " or " -> " moved the parsed link target, so an absolute hardlink could pass. The header listing now escapes spaces in names, which leaves the raw delimiter unambiguous. It also uses -P so absolute targets are listed as published. - Directory-mode restore took the last whitespace field as the name. It now keeps the full escaped name, which tar -T unquotes. - The absolute-hardlink fixture was built but never run. It now runs, alongside the new delimiter-in-name and spaced-directory fixtures. - The script header says why this is bash. Removing -P, the space escaping, or the full-name fix each fails a fixture. The rootfs erofs is still byte-identical (eca6ee5e). Spec-impact: none. Refs RIG-3850 Co-authored-by: Matt Wilkinson <matt@rigel.build>
restore-dir-modes feeds directory names to tar -T, which reads a line starting with "-" as an option. A layer directory named "--directory=.." passed every existing check and retargeted the mode restore outside the rootfs. check_headers now rejects any member whose name starts with "-". Fixture e8 fails without the check. The rootfs erofs is still byte-identical (eca6ee5e). Spec-impact: none. Refs RIG-3850 Co-authored-by: Matt Wilkinson <matt@rigel.build>
Both header parsers stripped the five-field prefix with a greedy space match, so a leading space in a name vanished. " --directory=.." then reached tar -T as an option, and any directory named with a leading space lost its restored mode. The strip now consumes exactly the single space after the time field. check_headers also rejects names starting with an escaped space. Fixtures e9 (leading-space option name) and f2 (leading-space dir mode) fail without the respective fix. The rootfs erofs is still byte-identical (eca6ee5e). Spec-impact: none. Refs RIG-3850 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
marked this pull request as ready for review
September 26, 2026 01:27
A formatter pass reflowed compass-guest-initrd, which this change does not touch. Restore the original layout so the diff stays on the unpack. Spec-impact: none. Refs RIG-3850 Co-authored-by: Matt Wilkinson <matt@rigel.build>
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
1 participant
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.
Closes two of the three deferred findings on guest rootfs assembly.
assemble-layers.shrejects any entry that is not a regular file, directory, symlink or hardlink. It also rejects a hardlink that is absolute or has a..component, a symlink to an absolute path outside/nix/store, and a relative symlink that climbs above the archive root. Across all 120 pinned layers the real image ships 18,367 files, 4,241 directories, 2,954 symlinks (790 absolute, all under/nix/store), no escaping relative symlinks, and no hardlinks, so the rule takes nothing away.guest-image/assemble-layers.sh, so the rootfs build and its tests run the same code. The published rootfs is byte-identical before and after:sha256 eca6ee5ec8b17ae5…on both builds.compass-guest-assembly-testsbuilds tiny deterministic layers in the sandbox. It covers whiteout-then-re-add, an opaque directory with same-layer siblings, a layer replacing a symlink parent with a directory, the contract resolving through an in-image/nix/storelink, a missing contract binary, a FIFO, an escaping symlink, a host-path symlink and an escaping hardlink. Each guard was red-checked: disabling it made its fixture fail. It runs asguest-image:test, andcidepends on it.The redirect finding stays open. curl's
--locationdrops theAuthorizationheader on the cross-host 307 topkg-containers.githubusercontent.com(verified withcurl -v), and the bearer is GHCR's anonymous constant, so there is no credential to leak.Spec-impact: none. Refs RIG-3850
Co-authored-by: Matt Wilkinson matt@rigel.build