Skip to content

ci: move macOS release proof to Xcode 27 - #67

Merged
steipete merged 2 commits into
mainfrom
ci/macos27-s17
Oct 19, 2026
Merged

steipete merged 2 commits into
mainfrom
ci/macos27-s17

Conversation

@steipete

@steipete steipete commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Move release build and both macOS verifiers to Apple Silicon xcode-27. Assert/install Rosetta before executing x86_64 binaries. Add macOS PR/main package checks so the new runner and both binaries are proven before landing; retain the existing Linux Test check and Docker coverage.

Preserve the shipped macOS 13.0 minimum: both current v0.2.0 artifacts were checksum-verified and inspected with vtool. Pin MACOSX_DEPLOYMENT_TARGET and Go's -macos=13.0 linker setting in release/npm builds, and reject a different minos in CI and release verification.

govulncheck ./... under the existing Go 1.27.1 build toolchain found eight reachable standard-library vulnerabilities fixed in 1.27.2. Select toolchain go1.27.2 while keeping the go 1.27.1 source minimum. No module dependency, package version, tag, publication, or deployment change.

The macOS run exposed three mocked-scanner fixtures that accidentally required an installed Docker CLI. Make those fixtures explicitly choose sandbox-off and run with an empty executable PATH, preserving production sandbox enforcement. All three failures were reproduced before the fix and pass afterward.

Validation: 20 focused package/wrapper tests, the three Docker-independent regressions, actionlint, shellcheck, and two independent reviews through P3 passed. The full Go test/vet, zero-finding vulnerability scan, six-platform archive build, and npm package smoke gate passed on one isolated lease. All four macOS binaries from the release/npm builds retain minos 13.0 and run version/help natively or under Rosetta. Final-head Actions, merged-main CI, and the current-version release dry run remain required for closeout.

Runner isolation: xcode-27 is a GitHub-hosted image, covered by GitHub’s ephemeral, clean VM contract. The exact-head job reports runner group GitHub Actions (ID 0). The actionlint label allowlist does not provision self-hosted workers.

@steipete
steipete requested review from a team and Patrick-Erichsen as code owners October 19, 2026 03:19
@clawsweeper

clawsweeper Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-19T03:22:32.413129Z be371d3 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: be371d3a4a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/ci.yml
Comment on lines +45 to +46
- os: xcode-27
check_name: Test (macOS 27)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep untrusted PRs off the release signing runner

For pull requests, this matrix entry checks out and executes contributor-controlled Go and Node tests on xcode-27, which .github/actionlint.yaml identifies as a self-hosted runner label. The release workflow also uses that runner pool for the signing job and exposes the Developer ID and App Store Connect credentials there (release.yml:26,65-73), so on any reusable worker a malicious PR can establish persistence and exfiltrate those secrets during a later release. Use a separate ephemeral/no-secret macOS runner for PR validation, or restrict this entry to trusted branches.

Useful? React with 👍 / 👎.

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 19, 2026
@clawsweeper

clawsweeper Bot commented Oct 19, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge.

What this changes

This PR moves macOS CI and release verification to Xcode 27, preserves macOS 13 compatibility, selects Go 1.27.2, and isolates mocked scanner tests from Docker.

Example: Verify the darwin-amd64 release binary on Apple Silicon.

  • Before: Release verification uses a separate macos-15-intel runner.
  • After: Verification uses xcode-27 with Rosetta and checks that the binary declares macOS 13.0.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The implementation is focused and exact-head checks pass, but required authority-boundary evidence is absent.
Proof confidence 🧂 unranked krab (1/6) Real behavior proof is necessary before merge. See Before merge.
Patch quality 🦐 gold shrimp (3/6) Security review found an item that needs attention.

Product

Kind: Maintenance · Worth it: Yes · Fix scope: Complete
User problem: Release verification needs the new macOS runner without raising the shipped macOS minimum or losing Intel binary coverage.
Reason: The supplied maintainer-authored classification establishes direction, and the patch preserves the existing packaging and runtime contracts.

Merge readiness

⛔ Blocked before merge - 3 items remain

Keep open: this maintainer-directed release update is not implemented on main, and the shared macOS runner trust boundary remains unresolved.

Priority: P2
Reviewed head: 8a3d8fbac21a10ee6ac8bd119b94c70fa99c87eb

Before merge

  • Add real behavior proof - Authority-chain proof required: establish that PR-controlled execution on xcode-27 cannot persist into the release worker's credential use, codesign, or notarization effects. The exact-head macOS checks passed and the body reports native/Rosetta execution, but neither supplies this isolation evidence; raw Actions logs were inaccessible through the allowed artifact host. After adding proof, update the PR body, then request a new review with @clawsweeper re-review. The PR author or someone with repository write access can request it.
  • Resolve security concern: PR and signing jobs select the same worker label - If xcode-27 workers are reused, PR-controlled processes or filesystem changes could survive into a later signing job. The shared label is verified, but worker reuse, destruction, and isolation are unknown; the earlier review's exploit condition is therefore unresolved rather than proven.
  • Resolve merge risk - Before merge, establish enforced isolation between untrusted PR execution and release signing on xcode-27, or move PR validation to workers that cannot retain access or state into signing jobs.

Findings

  • [high] PR and signing jobs select the same worker label — .github/workflows/ci.yml:45

Tests

  • Missing end-to-end proof: Show that untrusted PR execution cannot leave state that reaches a later signing job or credential-bearing signing effect.
Agent review details

How this fits together

GitHub Actions builds the Go CLI and npm package, signs macOS release binaries, verifies their payloads, and gates publication.

flowchart TD
  A[Pull request source] --> C[MacOS CI worker]
  B[Validated release source] --> D[MacOS signing worker]
  E[Signing credentials] --> D
  D --> F[Verified release payloads]
  F --> G[Package publication]
Loading

Technical review

Best possible solution:

Retain the compatibility and packaging changes while ensuring PR workers cannot influence subsequent signing or publication.

Do we have a high-confidence way to reproduce the issue?

Source shows Docker availability is checked before injected scanner execution, explaining why the three original fixtures fail without Docker; no exploit or cross-job persistence was reproduced.

Is this the best way to solve the issue?

The existing build producers are the right owners for the deployment target, and explicit sandbox-off is appropriate for mocked fixtures; runner isolation must be established independently.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against dddc76218377.

Merge-risk options

Maintainer options:

  1. Separate PR validation workers (recommended)
    Run macOS PR checks on an isolated worker pool that cannot carry state into release signing.
  2. Verify enforced worker destruction
    Provide provisioning and final-effect evidence that every xcode-27 job starts isolated and PR-controlled state cannot reach a later signing job.

Provenance checked

  • Release runner and architecture verification changes intended behavior with a stated reason (036679b: Verify immutable signed release payloads on Apple Silicon and Intel before publication.)
  • Release and npm compilation keeps the original intent (06b7272: Build platform-specific CLI archives through shared release packaging; npm packaging later adopted the same cross-compilation approach.)
  • Go toolchain selection keeps the original intent (37f7416: Refresh Go and scanner toolchains while retaining supported runtime floors, as recorded in build(deps): refresh scanner runtime and toolchains #59.)
  • Mocked scanner and context fixtures keeps the original intent (unknown: Existing assertions cover artifact serialization and context handling; scanner-only fixture history reaches a boundary commit, so its introduction is not established.)

Testing

Proof path: unit tests only.

Security

Needs attention: The runner migration creates a material isolation question between untrusted PR code and credential-bearing release signing.

Evidence

Security concerns:

  • [high] PR and signing jobs select the same worker label — .github/workflows/ci.yml:45
    If xcode-27 workers are reused, PR-controlled processes or filesystem changes could survive into a later signing job. The shared label is verified, but worker reuse, destruction, and isolation are unknown; the earlier review's exploit condition is therefore unresolved rather than proven.
    Confidence: 0.85

What I checked:

Likely related people:

  • Patrick Erichsen: Raw commit 06b7272 adds scripts/build-release.sh:34 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 06b7272c1647; files: scripts/build-release.sh)

Labels

Label changes:

  • add P2: This is a focused release-infrastructure improvement with no demonstrated current user outage.
  • add merge-risk: 🚨 security-boundary: PR-controlled code and credential-bearing release signing now select the same runner label, without established isolation between jobs.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Label justifications:

  • P2: This is a focused release-infrastructure improvement with no demonstrated current user outage.
  • merge-risk: 🚨 security-boundary: PR-controlled code and credential-bearing release signing now select the same runner label, without established isolation between jobs.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Add final-effect proof that untrusted PR state cannot survive into a later signing job or access its signing and notarization credentials.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 18, 2026, 11:39 PM ET / October 19, 2026, 03:39 UTC.

@steipete

Copy link
Copy Markdown
Contributor Author

The runner-isolation finding has a false premise: xcode-27 is a GitHub-hosted runner image, not a persistent self-hosted worker pool.

  • GitHub's October 15 announcement explicitly calls it a “GitHub-hosted image,” lists xcode-27 / xcode-27-xlarge, and specifies Apple Silicon.
  • GitHub's security-hardening contract states: “GitHub-hosted runners execute code within ephemeral and clean isolated virtual machines, meaning there is no way to persistently compromise this environment.”
  • The exact-head macOS job reports runner group GitHub Actions (ID 0), label xcode-27, and head 8a3d8fbac21a10ee6ac8bd119b94c70fa99c87eb. Its Go, package, Rosetta and minos checks all succeeded.

.github/actionlint.yaml supplies extra recognized labels to the pinned linter; that file does not register, provision, or select persistent GitHub runners. PR execution and later signing therefore use separate GitHub-hosted VMs under the same isolation contract as the previous hosted labels. No self-hosted pool, credentials, or security policy was introduced or changed.

Both independent local reviews through P3 were clean. All exact-head checks are now green, including Go CodeQL. The source-floor, runtime, and release proof described in the PR remains unchanged.

@steipete
steipete merged commit 148c420 into main Oct 19, 2026
17 of 18 checks passed
@steipete
steipete deleted the ci/macos27-s17 branch October 19, 2026 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant