randomparity / randomparity/kdive

feat: eliminate the external-boot preflight decompressing pass

Open
#2,571 0 comments 0 reactions 0 assignees View on GitHub
area:build-install effort:L priority:P2 risk:daytime-only status:ready type:feature
Dominant language
Python
Stars
0
Forks
0
Avg merge
1h 26m
Merged PRs (30d)
311

Description

## Problem

`_scan_external_boot_archive` calls `_preflight_external_boot_archive` before it
opens the archive with `tarfile`. The preflight is a full decompressing pass: it
walks every tar header in order, discards every member payload, and enforces
three bounds on the way. For a 2 GiB bundle that is ~512 object-store requests
and one complete gzip decompress cycle spent purely on bounding what the second
pass is about to do.

Removing it would take the finalization from two decompressing passes to one.
This is the highest-risk of the three changes split out of #2495, because the
preflight is not redundant work — it is a security boundary that exists
specifically to run *before* `tarfile` sees the stream.

## Evidence

- Preflight implementation:
`src/kdive/build_artifacts/validation.py:642-689`. Its docstring states the
purpose directly: "Bound raw tar work before `tarfile` consumes GNU/PAX
extension payloads."
- Invocation order — preflight at
`src/kdive/build_artifacts/validation.py:560`, inside
`_scan_external_boot_archive`, three lines before
`src/kdive/build_artifacts/validation.py:563` opens
`tarfile.open(fileobj=..., mode="r|gz")`. The ordering is the guarantee: the
extension cap fires before `tarfile` allocates for the extension payload.
- The three bounds the preflight enforces, all of which must keep firing at the
same thresholds and on the same inputs:
- member ceiling `_EXTERNAL_BOOT_ARCHIVE_MAX_MEMBERS`:
`src/kdive/build_artifacts/validation.py:659-663`. Note this one is
**already enforced a second time** inside the streaming pass at
`src/kdive/build_artifacts/validation.py:565-568`, against the same constant.
Removing the preflight therefore does not remove this bound, it only changes
when it fires — the streaming check trips after `tarfile` has produced the
offending `TarInfo` rather than before. Scope this guard as a timing change,
not a reimplementation.
- GNU/PAX extension payload cap `_EXTERNAL_BOOT_EXTENSION_MAX_BYTES` over
`XHDTYPE`, `XGLTYPE`, `GNUTYPE_LONGNAME`, `GNUTYPE_LONGLINK`:
`src/kdive/build_artifacts/validation.py:665-678`
- raw padded-byte ceiling `_EXTERNAL_BOOT_ARCHIVE_MAX_BYTES`:
`src/kdive/build_artifacts/validation.py:679-686`
- Constants: `src/kdive/build_artifacts/validation.py:63`
(`_EXTERNAL_BOOT_EXTENSION_MAX_BYTES`), `:60`
(`_EXTERNAL_BOOT_ARCHIVE_MAX_MEMBERS`), `:61`
(`_EXTERNAL_BOOT_ARCHIVE_MAX_BYTES`).
- Existing regression that pins the ordering guarantee:
`tests/providers/local_libvirt/test_validate_external_artifacts.py:485-498`
(`test_external_boot_scan_bounds_pax_metadata_before_tarfile_consumes_it`) —
it lowers `_EXTERNAL_BOOT_EXTENSION_MAX_BYTES` to 32, builds a PAX archive with
a 64-byte comment header, and asserts the failure. Its name is the contract.
- Existing gzip-bomb regression:
`tests/providers/local_libvirt/test_validate_external_artifacts.py:549-560`
(`test_kernel_tar_scan_is_bounded_against_a_decompression_bomb`).
- **Coverage gap found while verifying this breakdown:** a repository-wide grep
for `_EXTERNAL_BOOT_ARCHIVE_MAX_MEMBERS`, `_EXTERNAL_BOOT_ARCHIVE_MAX_BYTES`,
"member limit", and "raw tar byte limit" across `tests/` matches nothing. Only
the extension cap has a test. Two of the three guards being moved are
currently unprotected by any regression — including the member ceiling, in
both of the places it is enforced.

## Expected

The preflight's decompressing pass is gone and external-boot validation performs
one decompressing pass over the archive, with all three bounds still enforced at
their current values and still firing before `tarfile` allocates for an
attacker-controlled extension payload.

Acceptance conditions:

1. These two tests pass **unmodified**:
`tests/providers/local_libvirt/test_validate_external_artifacts.py:485-498`
and `:549-560`. A change that requires editing either of them is a change to
the security contract, not a refactor of it, and is out of scope here.
2. Before the enforcement point moves, add the missing regressions for the member
ceiling and the raw-byte ceiling, and verify they bite — make a controlled
fault, observe red, revert. Moving an untested guard is moving it blind.
3. The `external-boot-evidence-v1` document is field-for-field identical for the
same object.

## Proposed approach

The bounds cannot simply be moved into the existing `tarfile` loop. `tarfile` in
`r|gz` mode reads and interprets a GNU/PAX extension header's payload as part of
producing the next `TarInfo`, so by the time the loop body at
`src/kdive/build_artifacts/validation.py:564` can inspect anything, the
allocation the cap exists to prevent has already happened. Closing that gap means
one of:

- reimplementing enough tar header parsing to interpose on extension headers
ahead of `tarfile` — trading a second pass for a hand-written parser on a trust
boundary; or
- reaching into `tarfile` internals to bound the extension read — coupling
validation to a private CPython interface across upgrades; or
- keeping a header-only preflight that does not decompress member payloads,
which reduces the pass's cost without removing it.

Evaluate all three and record the choice, with its tradeoff, in a decision
record. This piece should not be implemented as a straight refactor.

## Risk

**This is the highest-risk piece of the #2495 split and should be scheduled
last.** The other two children —
raising `_RANGE_CHUNK_BYTES` and folding the bundle sha256 into the scan pass —
capture most of the performance win without touching the security guard: between
them they remove one of the three passes outright and halve or quarter the
request count of the passes that remain. If this issue is later closed as not
planned, the performance work from #2495 is still substantially delivered. That
is a defensible outcome, not an abandonment, and it should be preferred over any
implementation that weakens the ordering guarantee at `validation.py:560` to get
the last pass.

## Non-goals

Operator-approved 2026-09-16, applying to every child split from #2495:

- Changing the `external-boot-evidence-v1` schema or any field value. That is
owned by a separate ADR on the ADR-0583 line; this work requires
value-identical evidence.
- Relaxing or re-tuning any security threshold
(`_EXTERNAL_BOOT_EXTENSION_MAX_BYTES`,
`_EXTERNAL_BOOT_ARCHIVE_MAX_MEMBERS`, `_EXTERNAL_BOOT_ARCHIVE_MAX_BYTES`,
`_EXTERNAL_BOOT_MEMBER_MAX_BYTES`). Only the enforcement point may move, never
the values. That is the entire scope of this issue and its hardest constraint.
- Raising `Semaphore(1)` (`src/kdive/services/runs/complete_build.py:47`). That
is a separate admission-control and capacity issue.
- Re-running the ppc64le proof on POWER hardware as an acceptance gate. Tracked
by `docs/debt/0015-ppc64le-finalization-measurement-unrun.md`.
- Optimizing validation paths outside external boot.

## Measurement record

This change removes one of the two decompressing passes, so the published proof
record
`docs/design/2026-09-14-external-build-finalization-measurement-2318-proof-record.md`
becomes stale for the scan phase — the phase that accounted for 112 153 ms of the
112 201 ms ppc64le total — once this lands.

Parent: #2495. Related: #2314.

CAMPAIGN-OCCURRENCE: b6d43c8c0535-e428eada-4e97-4a9e-b1b8-eace09097d9d source=#2495 sweep=#2495
CAMPAIGN-OCCURRENCE-RATIONALE: split of #2495 approved by the operator during campaign triage because the three changes have independent risk profiles

Contributor guide

Open the contributing guide

Research direction

Start at _scan_external_boot_archive and _preflight_external_boot_archive in src/kdive/build_artifacts/validation.py:560-689, then read the named regression tests in tests/providers/local_libvirt/test_validate_external_artifacts.py. Evaluate the three proposed enforcement strategies and record the tradeoff in a decision record. Done means the preflight decompressing pass is removed or its cost reduced as decided, all three bounds remain unchanged, missing member and raw-byte regressions are added, the two existing tests pass unmodified, and evidence remains field-for-field identical.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
32/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.