[tysonthomas9]

Add isolated macOS native auto-update with pinned CUP/CRX trust, signed/notarized updater embedding, verified CRX packaging, scheduled launchd qualification, and immutable GitHub test-prerelease publication. The real 158 → 159 local and GCP HTTPS Omaha/GitHub updates passed with actual 159 relaunch, preserved bookmark and platform trust.

Stack 3/3: targets codex/signed-macos-releases; merge the lower two layers first. Review the base-to-head diff for updater-only changes. Native patch 08 and updater-specific release-tool changes live in this layer.

The server, server tests and GCP deployment exist only in private https://github.com/BrowserOperator/omaha. They were removed from the browser repo. The optional qualification harness requires explicit --omaha-src pinned to the reviewed server commit; normal browser and manual-release builds do not require the private repository. See scripts/updater/README.md.

Validation after separation: extracted each layer independently; Python/JS checks; both native patch fixtures; shared immutable-input load; canonical pinned-server launch; 10 protocol/native CUP/CRX tests and 2 archive tests passed. Earlier end-to-end evidence is preserved in docs/chromium-macos-autoupdate-158-to-159.md. The split does not change compiled browser/native patch behavior.

Cross-repository consumer: https://github.com/tysonthomas9/test-github-pipeline/pull/7 depends on this top layer for update publication. Publication defaults off and never activates Omaha. Nothing is merged; production rollout, updater self-update, x86/universal, Windows/Linux and remote CircleCI execution remain pending. No keys, binaries, generated cloud endpoint or node_modules changes are committed.

Supersedes the updater portion of combined draft #101. docs/chromium-change-review.md maps the full stack.

Review/merge order: #102 → #103 → #104. The bottom targets the original review baseline; each later PR targets the branch below it. CircleCI PR #7 is a cross-repository consumer of the top layer.

[tysonthomas9]

@claude Please review this PR's incremental base-to-head diff for concrete bugs and regressions. Focus on native updater trust boundaries, canonical pinned private Omaha dependency, CRX packaging, launchd qualification, and GitHub prerelease publication. This is part of the #102 → #103 → #104 stack; review only this layer against its configured base, not the full cumulative stack. The original review baseline is intentional. Report actionable findings with file/line and severity; distinguish verified bugs from assumptions or unavailable environment settings. Review only: do not edit code, merge, publish, deploy, sign builds, or access credentials. Existing native update/signing evidence and this split's independent validation are described in the PR.

[tysonthomas9]

Requested Claude review (authenticated Claude CLI, tools disabled, incremental base-to-head diff only). The GitHub workflow request failed because the Claude Code GitHub App is not installed on this repository, so this is the actual CLI reviewer output. Findings below are static review observations and may require validation; they do not mean additional builds/tests ran. No code was edited by Claude.

PR #104 review: native macOS updater layer

These findings come from static reading of the incremental diff only. I could not run GN, a standalone DevTools checkout, or the private Omaha repo.

Supported findings

Sev	Location	Finding
High (if standalone DevTools builds on macOS are supported)	BUILD.gn:7-12	The root BUILD.gn now unconditionally references scripts/updater/native:browser_operator_update_verify on Mac. That target depends on //base, //components/client_update_protocol, //components/crx_file, //crypto and //third_party/zlib/google:zip. GN resolves deps of every target declared in the root file. A standalone devtools-frontend checkout lacks these directories, so gn gen would fail on macOS for normal frontend development. The README and review doc say normal frontend builds don't need updater pieces. Fix: gate the group on being inside Chromium (e.g. build_with_chromium), or move it out of the root file.
Medium	scripts/updater/probe_macos_baseline.py:22	root = Path('/Users/tyson/chromium/updater-test') is hardcoded. Its callers install_macos_test_baseline.py and qualify_macos_upgrade.py use Path.home(). On any other account, such as the CircleCI runner, the probe uses a different app, profile and evidence tree than its caller. It then fails or checks the wrong install. Fix: use Path.home(), or pass the root through.
Medium	scripts/updater/publish_test_update.py:30	Publishing accepts update_channel == 'local-test'. A loopback build has KSUpdateURL and the updater endpoint set to 127.0.0.1:18158. If a cloud catalog ever served that CRX, upgraded clients would be stranded off the feed. The docs say "Never publish loopback builds", but the code allows it. Fix: allow only cloud-test.
Medium	scripts/package_chromium_release.py:~199-206	For updater-enabled builds, smoke_test launches the app from the mounted DMG. The doc itself says this can re-register the test browser's ecp to a /Volumes/... path, or bootstrap the test updater's launch agent on the signing host. Packaging has no guard against or detection of that side effect. Later qualification refuses the bad registration, so the failure is safe but noisy. CI signing hosts would also get a user-scope test updater installed. Fix: snapshot and compare the test controller registration around the smoke test, or launch with updater registration suppressed.
Low–Med (latent)	patch 08, chrome/BUILD.gn:~537-538 (both versions)	The generic embedder branch (browser_operator_updater_config != "") hardcodes --keystone-base-tag=autoupdate-test-arm64. It ignores current_cpu, and any future production config would ship a test tag. Fix: derive the tag from the config scope.
Low	scripts/updater/frozen_input.py:31	git diff --binary HEAD excludes untracked files. Patch 08 creates components/update_client/configurator.cc. Unless the patch applier stages it, source_patch_sha256 and source_has_local_changes don't cover it. The same applies to the composition provenance.
Low	scripts/updater/probe_macos_baseline.py:66-73	With --launch-via-services, a failed close_browser.mjs is swallowed (CalledProcessError ⊂ SubprocessError), and there is no fallback kill. An orphan browser can then hold the profile or app during later stages. A missing node (which → None) raises an uncaught TypeError.
Low	scripts/updater/package_macos_update.py:69	args.output.mkdir() runs after hdiutil attach but outside the try, so a failure there leaves the DMG mounted. Separately, the CRX pin is derived from the supplied public key and never compared to the compiled crx_pkhash. A mismatched key pair would pass packaging and then fail on clients.
Low	scripts/updater/qualify_macos_upgrade.py (baseline_bookmark read)	It hardcodes corrected-baseline-verification/profile-sentinel.json, but install_macos_test_baseline.py --stage is configurable. A non-default baseline stage is compared against stale evidence.
Low	scripts/updater/omaha_checkout.py	git diff --quiet HEAD -- omaha ignores untracked files under omaha/, which the service module could import. The README's claim ("rejects modified tracked service code") is accurate, but the pin is weaker than it looks.
Low	.gitignore:63-65	The "Python build and verification caches" comment was deleted. **/__pycache__/ now sits under the "Ephemeral public endpoint/key" comment.
Nit	patch 08 request_sender.h	class Configurator; is inserted between the RequestSender doc comment and the class, so the comment now attaches to the forward declaration. In configurator.cc, the includes are out of order.
Info	docs/chromium-macos-autoupdate-158-to-159.md:~26-27, throughout	The committed docs contain a GCP project ID, a billing-account suffix, a keychain profile name, notarization request IDs and /Users/tyson/... paths. None are secrets, but consider scrubbing before the docs become non-draft. Most of the file is a session log, which is better kept out of the repo.

Needs verification (depends on upstream code not in the diff)

• components/update_client/request_sender.cc (patched ctor, ~L195-197): possible key lifetime issue. BuildSigner(config->GetCupSigningKey()) passes key->spki from a temporary that is destroyed after construction. If the 158/159 ML-DSA client_update_protocol::Cup keeps a span to the key instead of copying it, this is a use-after-free. Upstream always passed a static array, so it never needed a copy. Your passing CUP tests suggest it copies. Confirm in components/client_update_protocol/cup.h, or store the key in RequestSender.
• Patch 08 external_constants_default.cc: verifier downgrade not gated on the embedder config. The switch from CRX3_WITH_PUBLISHER_PROOF to CRX3 depends only on CRX_PKHASH being non-empty, not on browser_operator_updater_config. If upstream non-Google Chromium branding already defines a non-empty crx_pkhash, this patch drops publisher proof for builds without the config too.

No other concrete bugs found. Signing, composition and publish-recovery logic look consistent with the stated evidence.

[tysonthomas9]

Follow-up on the Claude review (commit 61976c7):

• Fixed the standalone GN regression by gating the qualification group on build_with_chromium. Actual standalone gn gen now passes (1264 targets from 439 files).
• Fixed the probe's hardcoded account path with Path.home().
• Restricted the publisher to verified HTTPS cloud-test releases. A loopback manifest is rejected before any GitHub call; the existing cloud-qualified payload remains accepted by the channel rule.
• Checked the upstream CUP lifetime question: both the constructor and header show an owning crypto::keypair::PublicKey, parsed from SPKI during construction, rather than a retained span. The temporary SPKI is not retained.
• Checked the verifier-default question: unconfigured upstream branding sets crx_pkhash empty; the custom config provides the pin. The default unconfigured branch keeps publisher-proof verification.

Other static review observations remain visible for review; these fixes do not constitute production promotion or cross-platform qualification. No macOS privacy changes or extra engine rebuild were performed.

[tysonthomas9]

Claude follow-up on the small review fixes (read-only CLI, tools disabled):

The remaining Git-tag validation gap identified below is now fixed with the recommended git check-ref-format in both the CI pre-build block and the direct publisher. Tests rejected .., trailing dot and .lock refs before build/publication. The CI wrapper also requires the HTTPS cloud-test GN config before building, closing the configuration question below. The broader findings from the original reviews remain available for user review; nothing is merged.

---

No regressions found in these fix diffs. One low-severity gap:

Low: the early tag check accepts some names that Git will reject (mac-release-shipping.sh, new tag check). The pattern ^autoupdate-test-[A-Za-z0-9._-]+$ lets through names that are invalid as Git refs:

• names containing .., such as autoupdate-test-a..b
• names ending in ., such as autoupdate-test-1.
• names ending in .lock, such as autoupdate-test-x.lock

These pass the shell check and then fail at tag or release creation, after the full build, signing and notarization. That is the late failure this fix was meant to prevent. Two ways to close it:

• add git check-ref-format "refs/tags/$UPDATE_TEST_TAG" to the pre-build block, or
• tighten the pattern, for example forbid .. and require the name to end in [A-Za-z0-9].

Checked and fine:

• BUILD.gn gate: you confirmed standalone generation, and Chromium defines build_with_chromium = true. The relative dep path still resolves from the file's own directory.
• Path.home(): behaves the same for the original user, and is portable for others.
• cloud-test-only publishing: the check is stricter, and its error message matches.
• Quoted UPDATE_TEST_TAG: an empty parameter default still trips :?, because :? rejects empty values as well as unset ones.
• [[ =~ ]] with an unquoted pattern: works in Bash 3.2. In the bracket expression, . and a trailing - are literal.
• :? and -r checks before the build: the later use sites are unchanged, so the move is safe as long as the script does not cd between validation and packaging. That only matters if a key path is relative; it is not shown in the diff.

Not verifiable from the diff: the shell pre-checks do not confirm that the build will produce update_channel: cloud-test. If a CI build with PUBLISH_TEST_UPDATE=true can produce a local-test manifest, publish_test_update.py now rejects it only after build and notarization. Check that the CI publish path always selects cloud-test.

[tysonthomas9]

Interactive retest now PASSED after resetting only the isolated controller registration to the restored 158 install. The user triggered the check; native desktop About now shows 159.0.8144.0 (Official Build) (arm64) and up to date. Installed/registered versions and path match. The saved baseline bookmark's on-disk GUID/title/URL are preserved; strict signatures, stapled ticket and Gatekeeper pass. No privacy permissions changed.

The original misleading screenshot came from 158 running from the read-only mounted DMG while the shared test updater registration pointed at an already-upgraded 159 copy. The server was receiving 159 and correctly returned noupdate. The reset helper now supports preparing a repeat test without triggering About and waits for asynchronous registration persistence. Full local result: /Users/tyson/chromium/updater-test/interactive-158-to-159-result.json.