fix: fix codesigning bug#1619
Open
JKFerland wants to merge 1 commit intodifferent-ai:devfrom
Open
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
@JKFerland is attempting to deploy a commit to the Different AI Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
The following comment was made by an LLM, it may be inaccurate: |
src-opn
requested changes
Apr 30, 2026
Collaborator
src-opn
left a comment
There was a problem hiding this comment.
@OmarMcAdam Security-focused review completed.
Security issue:
apps/desktop/scripts/prepare-sidecar.mjs:269now suppresses a failedcodesign --force --sign -whenever the priorcodesign --remove-signatureexits non-zero. That does not prove the binary has an immutable linker-created ad-hoc signature; it also covers permission errors, malformed or corrupt signatures, file-system failures, and other unexpectedcodesignfailures. Because this path prepares executable sidecars, silently accepting a binary that was neither successfully re-signed nor positively verified weakens the macOS sidecar integrity gate.- Required fix: fail closed unless the script positively verifies the existing binary is already validly ad-hoc signed. For example, only bypass re-sign failure after checking the specific immutable-linker-signature condition and/or running
codesign --verifyplus inspecting the signature metadata. Otherwise keep throwing.
Checks performed:
- Pulled PR 1619 into a dedicated local worktree.
- Reviewed PR metadata: description includes summary, rationale, scope, testing notes, and closes #1618.
- Scanned the changed diff for hidden Unicode/control characters and malformed whitespace:
git diff --check, non-ASCII enumeration, control-character scan, and ASCII transliteration comparison all passed. - Reviewed the changed code path in context.
- Ran
pnpm install --frozen-lockfile: passed. - Ran
pnpm --filter @openwork/desktop prepare:sidecar: passed. - Ran
pnpm --filter @openwork/desktop check:electron: passed. - Ran
pnpm --filter @openwork/desktop build: passed.
Requesting changes until the code verifies the existing signature before suppressing the signing failure.
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
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.
Summary
linker-signed binary
Why
CS_LINKER_SIGNED ad-hoc signature embedded by the linker
error in Code Signing subsystem" for these binaries — the kernel
treats the linker signature as immutable
silently swallowed the removal failure, then crashed on the
subsequent --force --sign - with "main executable failed strict
validation"
(Signature=adhoc), it's runnable without re-signing
Issue
Scope
remove.status === 0, meaning we only error if the binary was
successfully unsigned and then signing failed — a real problem. If
removal also failed, the binary had an immutable signature and is
still valid.
Out of scope
or non-darwin platforms
Testing
Ran
pnpm devResult
CI status
Manual verification
Evidence
Risk
signature. A truly broken/unsigned binary (where --remove-signature exits 0) still throws.
Rollback