Skip to content

[2/3] Build and ship signed notarized macOS releases - #103

Draft
tysonthomas9 wants to merge 1 commit into
codex/chromium-integrationfrom
codex/signed-macos-releases
Draft

tysonthomas9 wants to merge 1 commit into
codex/chromium-integrationfrom
codex/signed-macos-releases

Conversation

@tysonthomas9

@tysonthomas9 tysonthomas9 commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator

Create a repeatable official ARM64 release with PGO, full frontend typechecking, nested Developer ID signing, notarization/stapling, APFS DMG packaging and required mounted signature/Gatekeeper/functional verification. The APFS path fixes the observed HFS packaging signature damage.

Stack 2/3: targets codex/chromium-integration and depends on that PR. Review this layer's base-to-head diff. It contains manual-release tooling only; the native updater and its signing/qualification code are added by the next layer. Shared immutable input/build validation has no updater import or private-repository dependency.

Validation: extracted this layer independently, checked imports and CLI entrypoint, confirmed shipping settings accept manual builds and reject updater-enabled builds, ran Python/JS checks, and applied/reverse-checked both native patch series. The same manual signing/APFS flow previously produced the verified signed/notarized 158 DMG. No new full Chromium compilation was needed for this organization change.

No merge or new release. Supersedes the release-tooling portion of combined draft #101.

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

Copy link
Copy Markdown
Collaborator Author

@claude Please review this PR's incremental base-to-head diff for concrete bugs and regressions. Focus on official build settings, nested signing, notarization, APFS packaging, immutable inputs, and mandatory platform/functional verification. 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

Copy link
Copy Markdown
Collaborator Author

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 #103 review: signed releases layer

I found no blocking or high-severity defects. The findings below are medium or low and come from reading the diff only. Items marked (env) depend on Chromium, macOS or Apple tooling behavior I couldn't run.

Medium

  1. The release manifest records source provenance at verification time and too coarsely. scripts/package_chromium_release.py, verify_release (provenance block near the manifest write).

    • In non-frozen mode, source_base_revision and source_has_local_changes come from the checkout as it is when verification runs, not when it was built.
    • The checkout uses --untracked-files=no and stores only a boolean. The applied patch series is uncommitted and the toolbar PNGs are untracked, so the manifest can't identify which patches or assets shipped.
    • Fix: record a hash of the source diff and the patch-series identity in browser-operator-build-settings.json during build_chromium_release.py, then copy that into the manifest.
  2. --verify-existing depends on the current build tree, not the release being verified. scripts/package_chromium_release.py, main.

    • The expected version comes from build/Browser Operator.app.
    • Non-frozen mode also runs require_current_build, which may run ninja build.ninja and so changes the build dir during a verify-only step.
    • The docs say the active checkout is now 159. A resumed 158 verification would therefore fail on a version mismatch or on stale build products.
    • Fix: take the expected version from the output or DMG, or require --frozen-input, and skip the currency check when only verifying.

Low

  1. Gatekeeper rejection evidence is lost. In assess_gatekeeper, run(..., check=True) raises on a nonzero spctl exit before evidence.write_text. Run with check=False, write the evidence, then fail.

  2. A volume detach in finally can hide the original error. verify_release runs run(['hdiutil','detach', volume]) with check=True. If a leftover browser helper keeps the volume busy after a failed smoke test, the CalledProcessError replaces the real failure. Consider retrying the detach or using -force, and logging the error rather than raising it.

  3. A failed mount lookup leaves the image attached. In both chromium_apfs_dmg.py and verify_release, the next(...) mount-point lookup sits outside try. If it raises StopIteration, nothing detaches the image, and main doesn't catch that exception either.

  4. find_identity only matches exactly. requested in entry tests tuple membership, so --identity must equal the full fingerprint (case-sensitive) or the full name. A partial name or a lowercase hash gets the misleading "exactly one identity" error.

  5. The Git wrapper can produce an invalid fetch. scripts/chromium_sync_git/git adds --depth=1 whenever --depth is absent. If depot_tools ever passes --deepen, --shallow-since or --shallow-exclude, the combined command is rejected by Git. Treat those flags as explicit depth.

  6. sync_chromium_release.py has weaker error handling than the other scripts.

    • It doesn't wrap main() in the RuntimeError/CalledProcessError handler they use, so errors print a traceback.
    • It unconditionally overwrites BROWSER_OPERATOR_REAL_GIT and DEVELOPER_DIR with the Xcode path, unlike build_chromium_release.py, which uses setdefault.
  7. build_chromium_release.py validates --gn-config too late. The check runs after the PGO and V8 profile downloads. Move it next to the other argument checks.

  8. Docs are inconsistent with the Beta correction and may link outside this layer.

Assumptions I could not verify (env)

  • Chromium signing pipeline: the exact pipeline.py replacement string, the _package_dmg locals (paths, dist, config, dmg_path), and the internal_config.py hook. Your apply and run results support these, and the script fails closed on a mismatch.
  • Dropped DMG extras: the APFS adapter omits whatever pkg_dmg used to add, such as background, .DS_Store and volume icon. This is cosmetic for unbranded builds.
  • WebUI private members: currentUpdateStatusEvent_ and getUpdateStatusIcon_ in verify_chromium_about.mjs, and the minified registry heuristic in verify_chromium_release.mjs. These could break silently on the next roll.
  • Official build with ThinLTO off: is_official_build=true with use_thin_lto=false and chrome_pgo_phase=2 on Mac built successfully per your report. Because of symbol_level=0 and enable_dsyms=false, shipped crashes can't be symbolized; that is a product decision, not a bug.
  • Machine-specific defaults: tthoma9 keychain profile, /Applications/Xcode.app and /Users/tyson/.... These are fine locally but will need changing for CI.

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.

1 participant