[Bram910]

What

Refuse unsupported inner ELF versions before unwrapping a SELF container. The unwrapper currently accepts either version field without checking it, can replace an existing output, and reports success even though the relinker rejects that ELF.

Require both e_ident[EI_VERSION] and the complete 32-bit e_version to be 1, matching the relinker's existing checks and the ELF header specification. Report both received values through the existing Refused diagnostic. The separate ABI-version byte is not restricted or rewritten.

Tested

• Based on main fa515def85; original implementation head 74285a827e. Review cleanup changes tests only; production code is unchanged. Python tooling only. No title, full-runtime, GPU or local macOS run.
• Review cleanup: compare the parser's entire refusal message exactly, so object 1 cannot match object 16777217. Remove the duplicate kernel-magic loop from CLI rejection coverage, reducing these subprocess cases from 24 to 12. Separate accepted-version and existing kernel-magic tests still cover both supported SELF magics.
• Red/green after cleanup: the two rejecting regression methods produce 31 failed subcases against main's unchanged parser (19 parser rejection combinations and 12 CLI rejection/output-preservation combinations). The six accepted ABI-version conversions pass. A diagnostic mutant reporting object 16777217 instead of object 1 passes the old parser assertion but fails all three affected revised parser subcases on both Windows and Linux.
• Review cleanup head 05bd732eaf2418deb8bc398a716bc76ce85885ef: full suite 54 run, 50 passed, 4 existing skips on Windows Python 3.13.5, and 54 run, 53 passed, 1 Windows-only skip on Ubuntu 22.04/WSL Python 3.11.0rc1. All three adjusted version methods also pass five consecutive runs on Linux Python 3.10.12.
• The registered Windows unwrap_self CTest passes three consecutive runs (19.92-20.39 seconds each), without timeout changes. These focused runs validate the test-only cleanup; the full relinker build/CTest results below describe the original implementation head.
• Windows x64, Python 3.13.5, MinGW-w64 GCC 14.1.0 UCRT, Release, ANYPS5_RELINKER_ONLY=ON, BUILD_TESTING=ON: cmake --build build-relinker --parallel 4 and ctest --test-dir build-relinker --output-on-failure --parallel 4; 240/240 CTest entries pass. Direct python tools/tests/test_unwrap_self.py: 54 tests run successfully, four existing platform/permission skips.
• Ubuntu 22.04 under WSL, installed alternate Python 3.11.0rc1: python3.11 tools/tests/test_unwrap_self.py; 54 tests run successfully, one Windows-only ACL skip. The default Python 3.10.12 runs all three new methods successfully five consecutive times. Its full suite has four pre-existing failed assertions in three I/O failure-injection methods, reproduced unchanged with main's parser. Those tests patch io.open, but this Python's Path.open calls its cached accessor instead. fix(tools): open the unwrap_self temporary file through io.open #10586 fixes that separate I/O failure-injection issue; it is not a prerequisite for ELF-version validation and is not included here. No unrelated tests or skips were changed.
• Rejected inputs leave the source unchanged and preserve an existing output or leave no new output or temporary files. Cases include zero versions, future versions, mismatched version fields, high 32-bit values with low byte 1, both SELF magics and valid ELF versions with ABI-version bytes 0, 1 and 255.
• Compatibility with fix(tools): make the scripts that start with a shebang executable #9547, which makes shebang scripts executable: both merge orders succeed and produce the same tree. No dependency on that file-mode change.
• python tools/check_conventions.py --base origin/main --head HEAD and git diff --check origin/main...HEAD pass.
• Open-PR duplicate searches found no inner-ELF version fix. python tools/pr_overlap.py --branch fix/unwrap-elf-versions reports no overlap with open pull requests.

Checklist

• Based on current main; no other open PR implements the same functions
• One topic per PR; follow-ups go in a new PR
• No comments in code except technical debt
• Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
• No notes, investigation .md files or images added to the repository (attach them to this PR)
• New third-party code is a submodule built from source
• Generic behaviour, not specific to one title
• Depends on: none
• Fixes: none
• Unblocks: none
• GPUs tested: n/a
• AI-assisted: yes, Codex

[MarcZX10]

Reviewed the parser change, rejection diagnostics, output-preservation cases, and ELF header field offsets. The guard correctly checks both EI_VERSION and the full 32-bit e_version, while leaving EI_ABIVERSION separate and unchanged. In an isolated checkout at 74285a8, I ran the three new unittest methods (test_unsupported_inner_elf_versions_are_refused, test_unsupported_inner_elf_versions_preserve_output, and test_current_elf_versions_preserve_the_separate_abi_version): all passed. Hosted conventions, intake, eligibility, Linux/Windows/macOS relinker, and ASan checks pass; full product builds are skipped for this tools-only change.

[THEMANnt]

Reviewed PR #10580 at head 74285a827eea3a9371e924b0029c1661d5da60a5 against pinned base fa515def85cda0b9f3b7a70764c2ed7da8167133. No actionable findings in this focused review.

Previously, parse_elf() accepted unsupported inner ELF versions and conversion could replace an existing output. It now requires both identification byte 6 and the complete 32-bit object version to equal 1, matching the inspected relinker checks. Validation follows header-length and byte-order checks and precedes output creation. The separate ABI-version byte remains unchanged. Tests cover 19 invalid parser combinations, 24 CLI rejection/output-preservation subtests across both SELF magics, and six successful ABI-preservation conversions. Existing assertions remain intact, with fresh fixtures and actual CLI subprocess execution.

The controller's local execution of the tools/tests/test_unwrap_self.py unittest suite on Windows AMD64, Python 3.11.9, recorded 51 baseline tests and 54 tests each for pristine PR and candidate, with four skips in each run. Candidate tests against baseline production code produced 43 assertion failures: unsupported versions were accepted instead of refused. Recorded whitespace and conventions checks also succeeded.

GitHub-reported checks succeeded for relinker-asan, relinker (ubuntu-latest, g++), relinker (windows-latest, g++), both named macOS relinker jobs, eligibility, intake, labels and conventions. Checks named windows, linux, write-watch and register-context were skipped; those are not passing product tests. No superseded executions appear in the snapshot.

The author's compiler/CTest and WSL results remain author-reported, separate from these local and GitHub results. Local POSIX permission/group and symlink behavior was skipped, and no game, GPU or full-runtime validation was performed. The evidence supports approval of this focused change.

[roboterspinne]

Reviewed head 74285a8 merged onto upstream/main 6b496aa (2026-10-24); the merge is clean, also together with #10586 (same file, the io.open change), check_conventions.py passes and pr_overlap.py reports no overlap.

Environment: Windows 11 Home 10.0.26200, Python 3.13.9 (Anaconda); Python only, no build needed.

Results

• python tools/tests/test_unwrap_self.py on the merged tree: 54 tests, 50 pass, 4 skipped, 23 s. Red/green: against main's unwrap_self.py the two rejecting tests fail in exactly 43 subcases (19 parser combinations, 24 CLI combinations), as the body counts; the ABI-version test passes on main, so it is coverage of unchanged behaviour. All three pass on the branch.
• The relinker side matches: ElfReader::ReadHeader (core/relinker/relinker/src/parsing/ElfReader.cpp:97-102 on main) refuses e_ident[EI_VERSION] != 1 ("Expected little-endian ELF64 version 1") and e_version != 1 ("Unsupported ELF object version: expected 1"), so an unwrapped ELF with either field off was always refused one step later; the unwrapper now says so itself and reports both values.
• Offsets check out against the ELF64 header layout: identification byte 6, e_version at 0x14, the ABI-version byte 8 untouched.

Nits

• test_unwrap_self.py:676: the assertRaisesRegex pattern ends after object {version} and is not anchored, so for version=1 it also accepts a message ending in object 16777217 or object 16777216; a message that printed e_version big-endian would pass all three version=1 parser subcases, and only the exact assertIn of the CLI test catches it. Ending the pattern with \) (or comparing str(exc) exactly) makes the parser test stand on its own; no escaping is needed, the literal parts contain no metacharacters.
• test_unwrap_self.py:682: the CLI test is the slowest of the suite (24 subprocess runs, about 2.5 s here); the SELF_MAGIC_KERNEL half covers no new code, since the magic is read only at unwrap_self.py:249 before parse_elf runs. Optional trimming, the suite stays well under the 30 s rule.
• Body, Tested: the Python 3.10 note (four pre-existing failures because Path.open binds io.open at import) is accurate, and #10586 fixes exactly those four; "fixed by #10586" in that bullet would save the reader the search. With both PRs combined all 54 tests pass here.

Notes, nothing to change here: the match with the relinker holds for the two version fields; ElfReader::ReadHeader also refuses e_machine != 62 and e_ehsize != 64, and GuestImageReader.cpp:26-29 requires e_phentsize 56, which the unwrapper still accepts and passes through (a container with e_machine 0 unwraps with exit 0 here), so the same pattern is a candidate follow-up, not a gap in this PR's topic. The message format copies tools/import_audit.py:51 and reports both values, more than the relinker's two messages do; fine. tools/run_job_manager_reference.py:59 calls unwrap() and catches no Refused, so every refusal there, this one included, ends in a traceback; pre-existing. #10159 lists both files in GitHub's file list, but its blobs equal main's, a stale base rather than an overlap.

Not checked: Linux and macOS runs, Python 3.10 and 3.11 (the body's pre-existing io.open failures on 3.10, which #10586 addresses), the body's merge checks with #9547, real SELF containers.

I found no blocking issue: the two fields and offsets are right, the check sits before any output or temporary file is created, an existing output survives a refusal, nothing downstream reads or rewrites e_version, the relinker refuses the same inputs one step later, and the counter-check holds. For disclosure: I have open PRs (#10667 on tools/tests/test_suggest_patches.py, and relinker PRs #10679, #10658, #10646, #10607, #10596, #10564, #10403, #10402, #10401, #10391, #10390); #10401 and #10391 touch the relinker's test_elf_header.py (ELF type cases), none touches the unwrapper.

Written with AI assistance (Claude Code). I ran the tests above on my Windows 11 machine.

[roboterspinne]

Nit: the pattern is not anchored after object {version}, so for version=1 it also accepts object 16777217; a message printing e_version big-endian would pass all three version=1 subcases here, and only the CLI test's exact assertIn catches it. Ending the pattern with \) or comparing str(exc) exactly would make this test stand on its own.

[roboterspinne]

Nit, optional: this makes the test the slowest in the suite (24 subprocess runs, about 2.5 s here), and the kernel-magic half covers no new code, since the magic is read only at unwrap_self.py:249 before parse_elf runs. One magic would halve the time with the same coverage.

[Bram910]

Addressed the three nits in 05bd732eaf2418deb8bc398a716bc76ce85885ef:

• The parser test now compares str(Refused) exactly. An in-memory diagnostic mutant changing object 1 to object 16777217 passes the old assertion but fails exactly the three affected revised subcases, independently of the CLI test. Verified on Windows and Linux.
• Removed the duplicate kernel-magic loop from CLI rejection coverage: 12 subprocess cases instead of 24, still covering each rejected field/value and both absent/existing output. The six accepted ABI-version conversions and the pre-existing kernel-magic test retain both-magic coverage.
• The PR description now names fix(tools): open the unwrap_self temporary file through io.open #10586 as the fix for the separate, already documented Python 3.10 I/O failure-injection issue; no unrelated fix is added here.

Production code is unchanged. Full suite: Windows Python 3.13.5 runs 54 tests (50 pass, four existing skips); Ubuntu 22.04/WSL Python 3.11.0rc1 runs 54 (53 pass, one Windows-only skip). The three adjusted version tests pass five repetitions on Linux Python 3.10.12. Revised negative controls against unchanged main fail exactly 31 rejection subcases (19 parser plus 12 CLI), while accepted ABI-version cases pass. Conventions and whitespace checks pass.

The registered Windows unwrap_self CTest also passes three consecutive runs (19.92-20.39 seconds each), without timeout changes.

[roboterspinne]

@Bram910 Thanks, checked 05bd732 merged onto upstream/main 2fade7e (2026-10-24), Windows 11, Python 3.13.9: tools/tests/test_unwrap_self.py 54 tests, 50 pass, 4 skipped, 25 s; against main's unwrap_self.py the two rejecting tests fail in exactly 31 subcases (19 parser, 12 CLI), as the body says. The mutant check holds too: with the message changed to object 16777217 for version 1, the old pattern still matched all three version=1 combinations and the exact compare fails exactly those three. Production code is unchanged and CI is green on this head. All three points are done from my side.

Written with AI assistance (Claude Code).

[MarcZX10]

Re-reviewed exact head 05bd732eaf2418deb8bc398a716bc76ce85885ef. The cleanup strengthens the parser assertion to an exact diagnostic match, removes duplicated CLI subprocess cases, and keeps coverage for both supported SELF magics and the independent ABI-version byte. Local test_unwrap_self passes (50 passed, 1 skipped); diff-check and conventions pass. The reported Windows and Linux runs also verify the exact diagnostic and mutation check, and the hosted relinker matrix passes.