Skip to content

Harden updater rebuild environment#941

Draft
tjdonley wants to merge 4 commits into
ilysenko:mainfrom
tjdonley:codex/harden-updater-build-boundary-upstream
Draft

Harden updater rebuild environment#941
tjdonley wants to merge 4 commits into
ilysenko:mainfrom
tjdonley:codex/harden-updater-build-boundary-upstream

Conversation

@tjdonley

Copy link
Copy Markdown
Contributor

Summary

  • run automated updater install and package-build subprocesses with a cleared environment, private HOME/XDG/TMP directories, and an executable path restricted to trusted root-owned system directories plus a trusted packaged Node runtime
  • stop inheriting user PATH, nvm, Cargo home, Linux feature roots/config paths, and developer safety overrides across the updater build boundary
  • copy the saved enabled-feature selection into a private data-only input, validate every ID against the bundled feature catalog, and pin both rebuild stages to that catalog
  • make packaged-wrapper dependency preflight use the same trusted path and check the full install toolchain plus the selected native package builder

Security impact

This closes two updater trust-boundary issues before a rebuilt package crosses the pkexec boundary:

  1. A user-controlled executable path could influence updater package contents.
  2. Environment-selected Linux feature code could execute during an automated rebuild.

The rebuilt package is now derived only from the bundled builder/feature tree and explicitly validated inputs.

Upstream integration

This branch is replayed onto current upstream main at a8dbcb9. A conflict with 949b68e was resolved by keeping user-local, nvm, Cargo, and inherited PATH entries excluded from automated package rebuilds; those paths are incompatible with the privileged-package trust boundary addressed here.

Validation

  • fork PR Omit npm from requirements on Ubuntu if nodejs is installed #4: all five Linux CI jobs passed on the reviewed implementation
  • cargo fmt --all -- --check — passed after replay onto current upstream
  • git diff --check upstream/main...HEAD — passed
  • focused updater tests previously passed: 12/12 builder tests and 10/10 wrapper-apply tests; strict Clippy also passed

Local retesting after the upstream replay is blocked on macOS by current upstream updater/src/notify.rs using notify-rust .hint(...), which is unavailable from the selected macOS backend. This branch does not touch notification code or dependency selection, and upstream Linux CI is green at the exact base commit; this PR's Linux CI is the final platform gate.

@tjdonley tjdonley marked this pull request as ready for review July 13, 2026 15:16

@ilysenko ilysenko left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for the security hardening work. Before investing more time in this approach, please note that addressing the findings below would not guarantee that this PR will be merged. Similar updater trust-boundary work is already being developed by the maintainer, with a focus on a simpler architecture. This implementation adds substantial complexity and long-term support cost, so the project may ultimately choose the alternative design and supersede this PR.

If you would still like to continue with this implementation, please address the following blockers:

  • Run wrapper-source Git operations with a clean environment and a verified trusted executable, with coverage for poisoned PATH and Git configuration.
  • Pin packaged rebuild feature inputs to a verified root-owned installed bundle instead of the configurable builder_bundle_root.
  • Validate the canonical target and ancestry of every executable admitted to the trusted build PATH.
  • Provide a supported trusted toolchain path and align native package dependencies, bootstrap documentation, proxy/CA behavior, and Cargo caching with it.
  • Bump the updater crate version as required by CONTRIBUTING.md.

These issues currently leave bypasses in the intended pre-pkexec trust boundary and break the documented native auto-update setup.

Before requesting another maintainer review, please verify the complete change against CONTRIBUTING.md, run the relevant tests locally, perform an independent review of the full diff with a local LLM, and wait until every required CI check has completed successfully after the update.

@tjdonley tjdonley marked this pull request as draft July 13, 2026 15:53
@Yo-DDV Yo-DDV added risk: high Touches security, privileges, updates, persistence, or multiple package paths. type: security Public security hardening or a disclosed security concern. area: build and install DMG extraction, dependencies, native modules, or installation flow. area: updater Update detection, rebuild, install, rollback, or updater state. area: native packaging Debian, RPM, or pacman package build and installation. and removed risk: high Touches security, privileges, updates, persistence, or multiple package paths. type: security Public security hardening or a disclosed security concern. area: build and install DMG extraction, dependencies, native modules, or installation flow. area: updater Update detection, rebuild, install, rollback, or updater state. area: native packaging Debian, RPM, or pacman package build and installation. labels Jul 14, 2026
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.

3 participants