fix(macos): preserve device identity storage - #101105
Conversation
|
@clawsweeper re-run |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Codex review: needs maintainer review before merge. Reviewed July 6, 2026, 11:46 PM ET / 03:46 UTC. Summary PR surface: Source +4, Tests +28, Other +305. Total +337 across 5 files. Reproducibility: yes. source inspection plus the linked reports give a high-confidence reproduction path: current main and v2026.6.11 prefer App Group storage before legacy while shipped macOS entitlements omit App Groups. I did not independently run a full signed app relaunch in this read-only review. Review metrics: 3 noteworthy metrics.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the entitlement-aware fallback with identity plus auth copy-if-missing migration after exact-head merge handling, and keep a full signed OpenClaw.app relaunch plus gateway restart as release validation. Do we have a high-confidence way to reproduce the issue? Yes, source inspection plus the linked reports give a high-confidence reproduction path: current main and v2026.6.11 prefer App Group storage before legacy while shipped macOS entitlements omit App Groups. I did not independently run a full signed app relaunch in this read-only review. Is this the best way to solve the issue? Yes, this is the best fix direction: gate App Group storage on the actual entitlement, fall back to writable legacy storage for unentitled builds, and migrate valid identity/auth state before minting anything new. Provisioning App Groups alone is heavier and does not cover shipped unentitled installs already in the field. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 780ca1d25315. Label changesLabel justifications:
Evidence reviewedPR surface: Source +4, Tests +28, Other +305. Total +337 across 5 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (3 earlier review cycles)
|
|
Addressed the P1 upgrade-preservation finding and refreshed the proof for head What changed since the last review
Real behavior proof (refreshed, in PR body) Known gap: no signed macOS app relaunch or gateway restart on a provisioned App Group container was run here; the entitlement/container system APIs need a signed bundle. The driver injects only the App Group directory location; the loader, decoder, save, and migration selection are the production functions. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
Avoid selecting App Group identity storage when the running app lacks the matching application-groups entitlement, so unentitled macOS builds fall back to the legacy writable identity directory. Existing unrecognized identity files are now preserved instead of being replaced by a newly generated identity, matching the no-overwrite behavior already used for recognized invalid identity data.
Limit the App Group entitlement probe to macOS builds so iOS keeps its existing App Group path behavior. Update the state-dir identity regression to assert the new no-overwrite contract for invalid identity-shaped files.
Unentitled macOS builds now migrate a readable App Group device identity into the selected legacy store before falling back, so an upgrade preserves the existing device id instead of minting a new one.
711afa6 to
eab93b9
Compare
|
Addressed the rank-up move: added a real code-signed macOS entitlement proof for head The prior proof injected the Embedded entitlements ( Live output: So on a real signature the entitlement read flips exactly as the resolver expects: an unentitled build routes to legacy with the App Group migration source active; an entitled build keeps App Group storage. The App Group container resolves on macOS, so the migration source is genuinely reachable for a real unentitled build, not only under an injected flag. Coverage now: the real @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
…back The app group to legacy identity migration now carries the sibling device-auth store file, so an unentitled build keeps its stored device tokens (keyed by the migrated deviceId) instead of re-pairing. The process-immutable entitlement check is resolved once per process rather than creating a SecTask on every state-dir lookup, and the migration source API returns an identity+auth pair struct.
|
Maintainer fixup pushed: cb1ff87 (thanks @yetval — the entitlement gate + migration direction is right and stays as designed). What changed
Upgrade-cohort check (why the rest of the design is kept as-is): every shipped macOS build is unentitled ( Proof (fixup head)
|
|
Merged via squash.
|
* fix(macos): preserve device identity storage Avoid selecting App Group identity storage when the running app lacks the matching application-groups entitlement, so unentitled macOS builds fall back to the legacy writable identity directory. Existing unrecognized identity files are now preserved instead of being replaced by a newly generated identity, matching the no-overwrite behavior already used for recognized invalid identity data. * fix(macos): repair identity CI coverage Limit the App Group entitlement probe to macOS builds so iOS keeps its existing App Group path behavior. Update the state-dir identity regression to assert the new no-overwrite contract for invalid identity-shaped files. * fix(macos): migrate existing app group identity on legacy fallback Unentitled macOS builds now migrate a readable App Group device identity into the selected legacy store before falling back, so an upgrade preserves the existing device id instead of minting a new one. * fix(macos): migrate device auth store with identity on app group fallback The app group to legacy identity migration now carries the sibling device-auth store file, so an unentitled build keeps its stored device tokens (keyed by the migrated deviceId) instead of re-pairing. The process-immutable entitlement check is resolved once per process rather than creating a SecTask on every state-dir lookup, and the migration source API returns an identity+auth pair struct. --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
* fix(macos): preserve device identity storage Avoid selecting App Group identity storage when the running app lacks the matching application-groups entitlement, so unentitled macOS builds fall back to the legacy writable identity directory. Existing unrecognized identity files are now preserved instead of being replaced by a newly generated identity, matching the no-overwrite behavior already used for recognized invalid identity data. * fix(macos): repair identity CI coverage Limit the App Group entitlement probe to macOS builds so iOS keeps its existing App Group path behavior. Update the state-dir identity regression to assert the new no-overwrite contract for invalid identity-shaped files. * fix(macos): migrate existing app group identity on legacy fallback Unentitled macOS builds now migrate a readable App Group device identity into the selected legacy store before falling back, so an upgrade preserves the existing device id instead of minting a new one. * fix(macos): migrate device auth store with identity on app group fallback The app group to legacy identity migration now carries the sibling device-auth store file, so an unentitled build keeps its stored device tokens (keyed by the migrated deviceId) instead of re-pairing. The process-immutable entitlement check is resolved once per process rather than creating a SecTask on every state-dir lookup, and the migration source API returns an identity+auth pair struct. --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
Summary
The macOS identity store no longer chooses App Group storage unless the running bundle has the matching
com.apple.security.application-groupsentitlement. Unentitled macOS app builds fall back to the legacy Application Support identity directory instead of repeatedly using an App Group path they cannot reliably persist.To keep that fallback upgrade-safe, an unentitled build now migrates a readable existing App Group
identity/device.jsoninto the selected legacy store before generating a fresh identity. The same installation keeps its device id across the storage switch instead of presenting a new one.Existing unrecognized
identity/device.jsonfiles are also preserved in both the Swift and TypeScript loaders. The loader can still return a fresh in-memory identity when it cannot decode the file, but it no longer replaces existing identity bytes unless it is creating a missing file or performing a proven safe normalization.Root cause
apps/shared/OpenClawKit/Sources/OpenClawKit/DeviceIdentity.swiftselected App Group storage wheneverFileManager.containerURL(forSecurityApplicationGroupIdentifier:)returned a URL:On affected shipped macOS builds, that could select the App Group container even though the bundle did not carry the matching App Groups entitlement. When persistence at that path failed,
loadOrCreatereturned generated identities, leading to pairing churn.The Swift and TypeScript
loadOrCreateimplementations also treated an existing but unrecognized identity file like a missing file:That path generated and saved a new identity over existing bytes.
Fix
Swift now checks the running bundle entitlement before selecting App Group identity storage:
When the entitlement is absent, the existing legacy Application Support fallback wins. To make that switch upgrade-safe, an unentitled build first tries to migrate a readable App Group identity into the legacy store:
migrationSourceURLis the App Groupidentity/<file>and is non-nil only when the entitlement is absent and the App Group container resolves. Migration reads and re-saves an already-valid identity, so the same device id survives while future writes go to the store the build can actually persist. Both loaders still preserve existing unrecognized identity files and only persist a generated identity when the file is missing.Why this is the right boundary
The storage decision belongs in
DeviceIdentityPaths, because every Swift caller reaches identity storage through that path resolver. The no-overwrite guard and the one-time migration belong in the identity loaders, because Swift and TypeScript both ownidentity/device.jsonand must preserve existing state consistently. OpenClawKit has noopenclaw doctor --fixsurface, so the bounded migration at the load boundary is the owner for this preservation.This resolves the ClawSweeper P1 (
DeviceIdentity.swift: preserve existing App Group identities before falling back) by implementing the recommended maintainer option: preserve/migrate a valid readable App Group identity into the chosen legacy store before an unentitled build switches to legacy storage. It does not accept a one-time re-pair and does not require provisioning the app with App Groups.The TypeScript node loader has no App Group container (
resolveStateDirinsrc/config/paths.tsnever selects one), so this migration is macOS-only; the TypeScript side keeps only the no-overwrite guard.Verification
swift test --filter DeviceIdentityStoreTestsbuildsOpenClawKitand passes all 13 cases (10 existing plus 3 new: migrate-on-fallback, keep-existing-legacy, no-source-when-entitled) on Apple Swift 6.3.2 / arm64 macOS.git diff --checkpassed.54f0b2589b11(pre-migration head) and passes with this patch.Real behavior proof
Behavior addressed: An unentitled macOS build that already held a readable App Group
identity/device.jsonpresented a new device id after upgrading, because the entitlement-aware fallback read empty legacy storage and minted a fresh identity.Real environment tested: The production
DeviceIdentityStore.loadOrCreateandDeviceIdentityPaths.appGroupMigrationSourceURLwere driven on a real on-disk App Group + legacy layout on the pre-migration head54f0b2589b11behavior (single-argument loader, unchanged by this patch) and on PR headeab93b9f6b7a3e1569107b9d28ae27106d940886, on Apple Swift 6.3.2 arm64 macOS. Only the App Group container location was injected; the loader, decoder, save, and migration selection are the production functions.Exact steps or command run after this patch: Wrote a valid existing
identity/device.jsonunder the App Group directory, then invoked the production loader for an unentitled build with empty legacy storage: the pre-fix path viaDeviceIdentityStore.loadOrCreate(fileURL:)and the fixed path viaDeviceIdentityStore.loadOrCreate(fileURL:migrationSourceURL:), and read back the returned device id and files. Driven underswift test --filter AppGroupMigrationProofScratch, capturing the returned device id values to disk.Evidence after fix:
Observed result after fix: On identical inputs the pre-fix path returns a new device id and writes a fresh legacy file, while the fixed path returns the same device id the install already had, keeps it across a reload, and leaves the App Group file byte-for-byte unchanged.
What was not tested: The full OpenClaw.app relaunch and gateway restart lifecycle was not run here (that needs the built signed app plus a running gateway); the entitlement read and container resolution themselves are now exercised on a real code-signed binary in the section below. No full
pnpm buildwas run because the change is Swift-only.Signed macOS entitlement proof (real code signature)
The production entitlement gate (
DeviceIdentityPaths.hasAppGroupEntitlement, i.e.SecTaskCreateFromSelf+SecTaskCopyValueForEntitlement) was compiled into a Mach-O binary and ad-hoc code-signed twice withcodesign --sign - --entitlements: once carryingcom.apple.security.application-groups = [group.ai.openclawfoundation.app.shared]and once with an empty entitlements dict.SecTaskCopyValueForEntitlementreads the running binary's own embedded entitlements, so the real signature drives the result.FileManager.containerURL(forSecurityApplicationGroupIdentifier:)was also called live.Embedded entitlements read back from each signature (
codesign -d --entitlements):Live output from the signed binaries:
On a real signature the entitlement read flips as expected: an unentitled build resolves to legacy storage with the App Group migration source active, and an entitled build keeps App Group storage. The App Group container resolves on macOS, so the migration source is reachable for a real unentitled build, not only under an injected flag. Combined with the loader proof above, the full chain is covered end to end except the OpenClaw.app UI relaunch and gateway restart.
Related
Fixes #99283.
Related to #99522.
Distinct from #79715, which fixed Swift and TypeScript schema compatibility but did not make App Group selection entitlement-aware.