Skip to content

Harden CLI fallback paths and gate unvalidated UUEFI host mutations for alpha#264

Merged
P4X-ng merged 3 commits into
mainfrom
copilot/review-installation-cli-correctness
May 18, 2026
Merged

Harden CLI fallback paths and gate unvalidated UUEFI host mutations for alpha#264
P4X-ng merged 3 commits into
mainfrom
copilot/review-installation-cli-correctness

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 16, 2026

Description

Reviewed the install and CLI experience for alpha-readiness, with specific attention to DoD wrapper behavior and UUEFI safety. This change fixes incorrect CLI delegation behavior when pf is present but unusable, restores the missing pb alias, and gates UUEFI host-side mutating helpers that are not yet broadly validated.

  • CLI fallback correctness

    • phoenixboot and phoenixboot-dod now probe the pf runner before delegating.
    • When pf exists but cannot actually run, both CLIs emit curated, actionable install guidance instead of leaking the raw runner failure.
    • phoenixboot list and phoenixboot-dod list now fall back to useful wrapper/help output in broken-runner environments.
    • Restores the expected pb -> phoenixboot symlink.
  • UUEFI alpha safety

    • Gates uuefi-install and uuefi-apply behind PHOENIXBOOT_ALPHA_ALLOW_UNTESTED_UUEFI_HOST=1.
    • Keeps uuefi-report as the safe default alpha path.
    • Removes the unsafe BootX64.efi placeholder fallback from uuefi-install; host install now requires a real UUEFI.efi.
  • Coverage and docs

    • Adds focused shell coverage for:
      • main CLI / DoD CLI delegation failure paths
      • UUEFI host-helper alpha gating
    • Updates alpha/TODO/README docs to reflect the gated UUEFI host-side surface.
# Host-side mutating UUEFI helpers are now explicit opt-in for alpha
PHOENIXBOOT_ALPHA_ALLOW_UNTESTED_UUEFI_HOST=1 ./pf.py uuefi-install
PHOENIXBOOT_ALPHA_ALLOW_UNTESTED_UUEFI_HOST=1 ./pf.py uuefi-apply
  • Breaking change?
    • Breaking change - Does this PR cause a break in build or boot behavior?
    • Examples: Does it add a new library class or move a module to a different repo.
  • Impacts security?
    • Security - Does this PR have a direct security impact?
    • Examples: Crypto algorithm change or buffer overflow fix.
  • Includes tests?
    • Tests - Does this PR include any explicit test code?
    • Examples: Unit tests or integration tests.

How This Was Tested

  • bash components/core/scripts/testing/test-phoenixboot-cli.sh
  • bash components/core/scripts/testing/test-uuefi-host-helpers.sh
  • bash components/core/scripts/testing/test-uuefi-source.sh
  • bash components/core/scripts/testing/test-dod-helper.sh
  • Manual smoke checks of:
    • ./phoenixboot list
    • ./phoenixboot invalid_command_xyz
    • ./phoenixboot-dod list
    • ./phoenixboot-dod invalid_command_xyz

Integration Instructions

  • No special integration required.
  • For maintainer-only host-side UUEFI mutation during alpha, set PHOENIXBOOT_ALPHA_ALLOW_UNTESTED_UUEFI_HOST=1 explicitly.

Note

Medium Risk
Medium risk because it changes CLI task delegation/fallback behavior and adds safety gating to host-side UEFI mutation scripts, which can affect user workflows and system boot configuration.

Overview
Hardens CLI delegation when pf is present but not runnable. phoenixboot and phoenixboot-dod now probe pf version before delegating, and on failure fall back to curated wrapper output plus actionable install hints instead of surfacing raw runner errors.

Gates unvalidated host-side UUEFI mutations for the alpha. uuefi-install and uuefi-apply now refuse to run unless PHOENIXBOOT_ALPHA_ALLOW_UNTESTED_UUEFI_HOST=1 is set, and uuefi-install no longer falls back to BootX64.efi when UUEFI.efi is missing.

Adds regression coverage for the new DoD CLI fallback paths and the UUEFI alpha gate, restores the pb alias, and updates alpha/docs to reflect the gated UUEFI surface.

Reviewed by Cursor Bugbot for commit 6967f1b. Configure here.

Copilot AI linked an issue May 16, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Review installation, CLI, and correctness issues Harden CLI fallback paths and gate unvalidated UUEFI host mutations for alpha May 16, 2026
Copilot finished work on behalf of P4X-ng May 16, 2026 20:21
Copilot AI requested a review from P4X-ng May 16, 2026 20:21
@P4X-ng P4X-ng marked this pull request as ready for review May 18, 2026 05:14
@P4X-ng P4X-ng merged commit 710ea9e into main May 18, 2026
5 checks passed
@P4X-ng P4X-ng deleted the copilot/review-installation-cli-correctness branch May 18, 2026 05:14
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before autofix could start.

Reviewed by Cursor Bugbot for commit 6967f1b. Configure here.

echo

INSTALL_SCRIPT="$PROJECT_ROOT/scripts/uefi-tools/uuefi-install.sh"
APPLY_SCRIPT="$PROJECT_ROOT/scripts/uefi-tools/uuefi-apply.sh"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test references non-existent script paths at repo root

High Severity

INSTALL_SCRIPT and APPLY_SCRIPT are set to $PROJECT_ROOT/scripts/uefi-tools/uuefi-install.sh and uuefi-apply.sh, but the actual scripts live at $PROJECT_ROOT/components/core/scripts/uefi-tools/. The scripts/ directory at the repo root contains compatibility symlinks per scripts/README.md, but these symlink targets are not resolvable in the repository tree (confirmed by Read returning "File not found" for both paths). On a fresh clone without out-of-tree symlink setup, all three tests will fail to find the scripts and report spurious failures — the bash "$INSTALL_SCRIPT" invocations will error with "No such file" rather than exercising the alpha gate logic being tested.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6967f1b. Configure here.

Comment thread phoenixboot-dod
echo " - If the launcher exists but python deps are missing, run: pip install --user -e ./pf-runner" >&2
echo " - You can also point PF_PYTHON at a working Python 3 interpreter if needed." >&2
dod_audit_log "ABORT: pf runner unavailable for arguments: $attempted_args"
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicated probe functions across two CLI scripts

Low Severity

pf_runner_launcher(), pf_runner_probe(), and print_pf_runner_unavailable() are copy-pasted almost verbatim between phoenixboot and phoenixboot-dod. The only difference is an added dod_audit_log call in the DoD variant of print_pf_runner_unavailable. Extracting these into a shared sourced library (e.g., alongside includes/lib/common.sh) would avoid divergence risks when fixing bugs in the probe logic.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6967f1b. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Review installation, cli, correctness

2 participants