fix: keep framework symlinks so the recorder DMG can be signed - #216
Merged
Merged
Conversation
The frozen CLI embeds a Python.framework, whose canonical bundle layout uses symlinks (Versions/Current and the top-level Python/Resources). codesign needs those symlinks to recognize the framework; without them it aborts with "bundle format is ambiguous", so build-dmg.sh could not sign the release DMG. The install-time payload validator (CLIPayloadInspector) rejected every symlink, and freeze-cli.sh dereferenced the framework aliases into real files to satisfy it — which is exactly what broke signing. Instead, accept symlinks that are safe: relative and resolving inside the payload. Each alias is bound into the payload identity (a new link record) so a changed target still changes the fingerprint, and absolute or escaping targets are rejected. A symlink-free payload's identity is unchanged. The freeze step now validates the aliases in place (relative, in-payload, non-cyclic, not world-writable) and fails closed before signing, instead of dereferencing them. Renamed packaging/materialize-cli.py to validate-cli-payload.py to match.
…ared validator build-app.sh rejected any symlink in the frozen CLI, the same blanket rule as the installer and freeze step. Delegate to packaging/validate-cli-payload.py so all three enforce one contract (relative, in-payload, non-cyclic) and the framework keeps its codesignable symlinks. Give the validator a clean CLI error.
Adversarial review found the lexical containment check unsound. A symlink through a shallower-pointing intermediate symlink — pivot -> "." and E -> "pivot/../etc/passwd" — passed the lexical stack arithmetic yet physically escaped the payload, because ".." after a symlinked component resolves against that component's physical target, not the lexical path. That let a stable payload path alias external bytes whose contents the identity never hashes, defeating the tamper-evidence the validator exists to provide. Forbid ".." in symlink targets outright, in both the install-time validator (CLIPayloadInspector) and the build-time validator (validate-cli-payload.py). A relative target with no ".." component can only descend from its own in-payload directory, so it stays confined even through other forward-only symlinks. The framework's canonical aliases are all forward-only, so nothing legitimate is rejected. Adds tests for the through-symlink escape and a relative ".." escape.
The frozen-CLI symlink gate now accepts safe in-payload aliases and rejects unsafe ones via the shared validator, so assert both: a relative in-payload alias builds, an escaping alias is rejected with the validator message.
This was referenced Sep 14, 2026
Merged
mikehasa
added a commit
that referenced
this pull request
Sep 14, 2026
The app upgrades the recorder (CLI + daemon) on launch when it is newer than the installed one, but upgradeInstalledCLIIfNeeded returned .notNeeded identically whether the recorder was already current or could not be recognized at all. That silent no-op made an app-newer-than-recorder skew impossible to diagnose: nothing upgraded, and nothing said why. Add upgradeBlockedReason: when a recorder is present but no upgrade context can be built — the app cannot verify its own bundled recorder, or the installed recorder is not recognized as app-owned — name the reason. upgradeInstalledCLIIfNeeded records it on recorderUpgradeDiagnostic and logs it instead of skipping silently, and clears it once an upgrade proceeds. The reason is computed only inside the upgrade check (which already does the heavy identity/provenance work), never during rendering, so it does not traverse or hash payloads. Add the regression test that was missing: a legacy install whose Python.framework keeps its symlinks is auto-upgraded by a newer app (the shape #216 enabled and a pre-#216 inspector could not recognize), plus coverage for both blocked-reason branches (installed recorder unrecognized; bundled recorder unverifiable).
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
1 participant
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.
Fixes the DMG signing blocker tracked in #215.
Problem
The frozen CLI embeds a
Python.framework, whose canonical bundle layout uses symlinks (Versions/Current, and the top-levelPython/Resourcesaliases).codesignneeds those symlinks to recognize the directory as a framework; without them it aborts withbundle format is ambiguous (could be app or framework). Sopackaging/build-dmg.shcould not sign the release DMG, and no signed/notarized build could be cut. (This surfaced only on the signed release path — an unsigned local build and CI both skip signing, which is why it went unnoticed; #190 also never built a DMG because it did not compile until #212.)Root cause
Two contracts collided:
CLIPayloadInspector, the install-time payload identity validator, which rejected every symlink in the payload.materialize-cli.py, which dereferenced the framework's symlinks into real files so the payload would pass that validator — but a symlink-free framework is not a valid, signable framework, which is what brokecodesign.So the payload could either pass the installer's validator or be codesignable, but not both.
Fix
Accept symlinks that are safe instead of forbidding all of them, at both enforcement points:
CLIPayloadInspector.identity(install-time): a symlink is accepted only if its target is relative and contains no..component. A forward-only relative target can only descend from its own in-payload directory, so it stays confined even through other forward-only symlinks (the framework's canonical aliases are all forward-only). Each accepted alias is bound into the payload identity as a new link record (path + target), so retargeting an alias still changes the fingerprint. A symlink-free payload's identity is byte-identical to before (the change only adds a branch), so already-recorded identities and every existing payload test are unaffected.materialize-cli.py→validate-cli-payload.py.This preserves #188's real intent (no link can alias bytes from outside a stable path) while keeping the framework a codesignable bundle.
build-app.sh's own blanket symlink rejection now delegates to the same validator, so all three enforcement points share one contract.Adversarial review
An adversarial review of the first cut caught a real soundness bug in the install-time check: an earlier version resolved
..lexically, sopivot -> "."plusE -> "pivot/../etc/passwd"passed even thoughEphysically escapes the payload (..after a symlinked component resolves against that component's target, not the lexical path). The fix forbids..in symlink targets outright in both validators — provably confining and accepting every real framework alias — with tests for the through-symlink escape.Verification
pytest tests/test_packaging_validate_cli_payload.py— 12 passed (valid framework preserved; external / absolute / relative-../ through-symlink / broken / cyclic / fifo / world-writable rejected; linked root rejected).swift test— full macOS suite green (addsCLIPayloadSymlinkIdentityTests: accepts safe framework aliases; rejects absolute, relative-.., and through-symlink escapes; tamper-evident to a retargeted alias; distinguishes a symlink from a regular file at the same path).pytest -q(full backend suite) andswift build -c release— clean.packaging/build-dmg.sh --releaseon the release toolchain — produces a signed and notarized DMG (Apple notarizationAccepted, stapled), andpackaging/verify-dmg.shpasses:valid on disk,satisfies its Designated Requirement,notarized + stapled. This is the exact step that aborted before the fix.Related
materializeapproach from Redesign native setup and Work with a central activity timeline #190; unblocks the 0.10.8 release.mainbuilds on the release toolchain.