refactor(cli): harden macOS signature checks per #5675 review#5683
Open
avallete wants to merge 2 commits into
Open
refactor(cli): harden macOS signature checks per #5675 review#5683avallete wants to merge 2 commits into
avallete wants to merge 2 commits into
Conversation
Address three review points on the macOS code-signing change: 1. Exact identifier match instead of substring. Both the build-time verification (build.ts) and the macOS smoke-test helper now compare the full signed identifier, so a sidecar mistakenly signed as `com.supabase.cli` can no longer satisfy the `com.supabase.cli-go` check. 2. signDarwinBinaries no longer reaches into the module-level `shell`. It takes `shell` as a parameter and resolves the binary list via darwinBinariesForShell, so it stands on its own. 3. Single source of truth for identifiers. New apps/cli/scripts/macos-signing.ts exports MACOS_IDENTIFIERS + helpers, imported by both build.ts and the smoke-test helper. The redundant (and substring-matching) "Verify macOS signatures" workflow step is removed — build.ts already verifies each signature against the shared source and fails the build on mismatch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GnLjngbm48rMYVwn9Guduc
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@a1ca41eb9a95df44badb3c271117ba79ea988aa2Preview package for commit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the merged macOS code-signing change (#5675), addressing the three non-blocking review points from @Coly010.
Changes
Exact identifier match instead of substring. The previous checks used
includes("identifier: com.supabase.cli"), which also matchescom.supabase.cli-go— so a sidecar accidentally signed with the SFE's identifier would have passed. Both the build-time verification inbuild.tsand the macOS smoke-test helper now extract and compare the whole identifier value, so each binary is verified against exactly its own identifier.signDarwinBinariesno longer reaches into the module-levelshell. It now takesshellas a parameter and resolves its binary list viadarwinBinariesForShell(shell), so the function stands on its own and the legacy/next split lives in one place.Single source of truth for identifiers. New
apps/cli/scripts/macos-signing.tsexportsMACOS_IDENTIFIERSplusmacIdentifierFor()/darwinBinariesForShell()helpers, imported by bothbuild.ts(signing) and the smoke-test helper (verification). The third copy — the hardcoded, substring-matchingVerify macOS signaturesstep inbuild-cli-artifacts.yml— is removed:build.tsalready verifies each signature against the shared source during the build and throws on mismatch (failing the job), so the separate step was redundant and was the remaining drift/substring risk. If you'd prefer to keep an explicit standalone CI verification step, I can re-add one that imports the shared module instead of hardcoding the identifier — let me know.The sign-on-Linux / verify-on-macOS approach is unchanged.
🤖 Generated with Claude Code
Generated by Claude Code