Skip to content

Fix #753: [Bug][macOS] v1.2.3 → v1.2.4 内置更新失败:codesign requirement 缺少 "=" 与 "field." - #754

Closed
Memtensor-AI wants to merge 1 commit into
MemTensor:mainfrom
Memtensor-AI:bugfix/autodev-753-20261026235019878
Closed

Memtensor-AI wants to merge 1 commit into
MemTensor:mainfrom
Memtensor-AI:bugfix/autodev-753-20261026235019878

Conversation

@Memtensor-AI

Copy link
Copy Markdown
Collaborator

Description

Fixes issue #753: macOS in-app updates from v1.2.3 to v1.2.4 failed because the generated update helper passed a malformed codesign requirement.

Root cause and fix (App/shell/desktop/src/main/mac-update-scripts.ts): the requirement string handed to codesign --verify -R had two syntax errors — it lacked the literal "=" prefix (so codesign interpreted it as a requirement file path and reported "No such file or directory") and the Developer ID certificate extension OID lacked the field. prefix (parser error "unexpected char: '.'"). A new exported buildMacSigningRequirement() builds the literal requirement as =anchor apple generic and identifier "cn.memtensor.memmy" and certificate leaf[field.1.2.840.113635.100.6.1.13] exists and certificate leaf[subject.OU] = "<team>", and is now the single source for both the stage and install helper scripts. The existing bundle-ID, Developer ID extension, TeamIdentifier and full codesign --verify --deep --strict checks are all retained; the unsigned-build path is unchanged. Values containing quotes or backslashes are rejected instead of being spliced into the generated shell script.

Tests (App/shell/desktop/tests/mac-update-scripts.test.ts, gated to macOS, plus cross-platform assertions): (1) the generated requirement starts with "=", uses field.<oid>, and contains the bundle ID / subject.OU clauses; (2) both helper scripts embed the corrected literal and no longer contain the legacy forms; (3) unsigned builds omit the team requirement; (4) real codesign verification of ad-hoc-signed .app fixtures proves the corrected value is treated as an inline requirement (no ENOENT / "unexpected char"), reproduces both legacy failures exactly as reported, matches the bundle ID and rejects a different one; (5) a throwaway self-signed identity (TeamIdentifier in subject.OU + Developer ID extension OID, in a temporary keychain) confirms a same-team package passes the team/extension clauses while a foreign team, foreign bundle ID, or missing extension is rejected, and that the full shipped requirement parses (only the Apple anchor excludes the synthetic fixture). A mock-codesign test also asserts the helper forwards the literal requirement string to -R. App/shell/desktop/tests/packaged-runtime-boundary.test.ts was extended to lock in the new IPC channel.

UI (App/frontend/desktop): update failures now carry a showLogs flag, and the About tab renders a "查看更新日志 / View update logs" action beside the error instead of only "安装更新失败,请稍后再试。". New memmy:open-update-logs IPC + openUpdateLogs() preload bridge opens the updates directory containing mac-update-install.log, where the concrete reason is recorded. New zh/en message key settings.about.openUpdateLogs; settings-page test covers error, prepared-with-failure, success (no link), and English rendering.

Verification: tsc --noEmit clean for App/shell/desktop and App/frontend/desktop; eslint reports 0 errors (only pre-existing warnings); targeted suites pass — shell mac-update-scripts + packaged-runtime-boundary + windows-update-validation (115 passed, 28 macOS-skipped on this Linux runner), frontend settings-page + i18n + model-config parity (103 passed). Known unrelated pre-existing failures on this Linux runner: React act is not a function in several App/frontend/desktop/src/app interaction tests and a computer-use-agent-restart test (both reproduce on a clean tree); Electron-dependent shell suites cannot load electron on Linux. The new macOS-only codesign tests were not executed here and will run on macOS CI.

Related Issue (Required): Fixes #753

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (does not change functionality, e.g. code style improvements, linting)
  • Documentation update

How Has This Been Tested?

Automated tests are pending.

  • Unit Test
  • Test Script Or Test Steps (please provide)
  • Pipeline Automated API Test (please provide)

Checklist

  • I have performed a self-review of my own code
  • I have commented my code in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • I have created related documentation issue/PR in MemOS-Docs (if applicable)
  • I have linked the issue to this PR (if applicable)
  • I have mentioned the person who will review this PR

Reviewer Checklist

…ate logs

The macOS update helper passed a bare requirement expression to
`codesign --verify -R`, so codesign treated it as a requirement file path
("No such file or directory"), and the Developer ID certificate OID lacked the
`field.` prefix, which the requirement parser rejects at the first dot
("unexpected char: '.'"). Every signed in-app upgrade therefore failed at
staging with "Memmy update does not have the installed Apple Developer ID
identity", leaving the UI on a generic "try again later" message.

- Build the requirement through `buildMacSigningRequirement()`, which emits the
  literal `=` prefix and `certificate leaf[field.<oid>] exists`, while keeping
  the bundle ID, Developer ID extension, TeamIdentifier and full signature
  check. Values containing quotes/backslashes are rejected instead of being
  spliced into the shell script.
- Add real `codesign` coverage for the generated requirement on macOS: the
  corrected form parses and matches bundle ID, team and Developer ID extension
  via a throwaway self-signed identity, while the two legacy forms still fail
  with the reported errors and a foreign team/bundle ID is rejected. A mock
  codesign also asserts the helper now forwards the literal requirement.
- Show a "View update logs" action next to update failures. A new
  `memmy:open-update-logs` IPC opens the updates directory, where
  `mac-update-install.log` records the concrete failure reason.

Refs MemTensor#753
@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

🤖 Open Code Review

Target: PR #754
Task: d93c34215c89cf05
Base: main
Head: bugfix/autodev-753-20261026235019878
Head SHA: ac8d05f1abc66b64d375beb56e3d39fa1ae96d9f

🔍 OpenCodeReview found 6 issue(s) in this PR.


1. App/frontend/desktop/src/app/update-coordinator.tsx (L260)

showLogs: true is also set on the download failure path, but the updates directory only receives a log from the install/stage helper scripts (mac-update-install.log / win-update-install.log, written via exec >> "$LOG_PATH" in mac-update-scripts.ts and Write-MemmyUpdateLog for Windows). Failures raised earlier in downloadUpdate() — e.g. fetch rejecting, update package download failed: <status>, or validateWindowsDownloadedUpdate throwing — are never appended there, so in those cases the new "View update logs" button opens a directory with no failure record and the user is back to guessing.

Either restrict showLogs to the install catch below, or have the main-process download path append its error to the same updates log before rethrowing.


2. App/frontend/desktop/src/pages/settings-page.tsx (L1337-L1339)

All failure modes are swallowed here: if window.memmy?.openUpdateLogs is unavailable (browser runtime / older preload) the function returns silently, and if the IPC call rejects the user only gets a console.warn. The button stays visible and clickable either way, so "nothing happens" is indistinguishable from a broken bridge.

openDeveloperLogs right below handles the analogous case with setDeveloperFeedback; consider at least mirroring that pattern (unavailable + error message), otherwise the user clicks to diagnose an install failure and receives no feedback at all.


3. App/shell/desktop/src/main/main.ts (L6683-L6690)

openUpdateLogs is a byte-for-byte copy of openLogsDirectory except for the directory resolver (same mkdir + shell.openPath + error rethrow). Extracting one helper keeps the two paths from drifting (e.g. if directory creation or error handling is later hardened, it would otherwise have to be changed twice).

async function openDirectoryInFileManager(directory: string): Promise<void> {
  await mkdir(directory, { recursive: true });
  const openError = await shell.openPath(directory);
  if (openError) {
    throw new Error(openError);
  }
}

Then openLogsDirectory / openUpdateLogs become one-liners.

💡 Suggested Change

Before:

async function openUpdateLogs(): Promise<void> {
  const updatesDirectory = resolveUpdatesDirectory();
  await mkdir(updatesDirectory, { recursive: true });
  const openError = await shell.openPath(updatesDirectory);
  if (openError) {
    throw new Error(openError);
  }
}

After:

async function openUpdateLogs(): Promise<void> {
  await openDirectoryInFileManager(resolveUpdatesDirectory());
}

4. App/frontend/desktop/src/pages/settings-page.tsx (L1333)

This repeats the bridge-guard already implemented twice in this same file (openDeveloperLogs, exportDiagnosticsReport): typeof window === "undefined" check, typeof fn !== "function" guard, and promise handling. A fourth copy will drift (the existing ones already differ — openDeveloperLogs reports settings.developer.openLogsUnavailable and tracks a busy state, this one does neither). Extract a small helper such as invokeBridge(fn, onError) or reuse the developer-feedback state so all open/exports behave consistently.


5. App/frontend/desktop/src/pages/settings-page.tsx (L1906)

showLogs is being used as a styling signal here, which conflates two concerns: whether a log link should render, and whether the message is an error. Today they happen to coincide, but any future feedback that carries a log path without being a failure (e.g. "install started, see logs") would be rendered in text-status-error by mistake. Consider giving UpdateFeedback an explicit tone?: "error" | "info" field and driving the color from that, leaving showLogs purely about the link.


6. App/shell/desktop/src/main/mac-update-scripts.ts (L29-L30)

buildMacSigningRequirement guards teamIdentifier but not bundleIdentifier, even though both are interpolated into the same requirement expression. An empty/whitespace bundle id silently produces identifier "", which codesign rejects at install time with the generic "does not have the installed Apple Developer ID identity" message instead of an explicit generator error. Add the symmetric non-empty check so the failure surfaces where the requirement is built.

💡 Suggested Change

Before:

  if (!teamIdentifier) throw new Error("Mac signing requirement requires a team identifier");
  if (/["\\]/.test(teamIdentifier) || /["\\]/.test(bundleIdentifier)) {

After:

  if (!teamIdentifier) throw new Error("Mac signing requirement requires a team identifier");
  if (!bundleIdentifier.trim()) throw new Error("Mac signing requirement requires a bundle identifier");
  if (/["\\]/.test(teamIdentifier) || /["\\]/.test(bundleIdentifier)) {

Generated by cloud-assistant via Open Code Review.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

🔧 Open Code Review requested Agent fix

Open Code Review found 6 issue(s). I have resumed the development Agent to fix them.

  • Task: d93c34215c89cf05
  • Fix attempt: 1/2
  • Finding delta: 0 repeated / 6 new / 0 likely resolved

The Agent will push a new commit to this PR branch. OCR will recheck after the commit is pushed.

@syzsunshine219

Copy link
Copy Markdown
Collaborator

We are closing this PR and will not continue the review fixes here.

The macOS in-app update fixes are already on main: lipo receives the executable before -verify_arch, and the codesign -R requirement starts with = and uses the field.1.2.840.113635.100.6.1.13 OID. That change is 580413e, merged to main in PR 729, and it is also on release/v1.2.401. This PR is still based on 74b26fc and conflicts with current main.

The six review comments are about the additional "View update logs" work. The signature fix does not depend on that UI. If we want the log action later, we will open a new PR from current main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug][macOS] v1.2.3 → v1.2.4 内置更新失败:codesign requirement 缺少 "=" 与 "field."

4 participants