fix(trusty-installer): make trusty-memory and trusty-analyze signable targets - #5018
fix(trusty-installer): make trusty-memory and trusty-analyze signable targets#5018bobmatnyc wants to merge 3 commits into
Conversation
trusty-memory was missing from SIGNABLE_BINARIES for no recorded reason — no comment, doc, ADR, or commit message ever gave one. The table grew a row per reported symptom (#2558 search, #2721 tm, #2951 the GUI, #4277 tagent) and nobody filed the trusty-memory case. Owner ruled 2026-08-06 that it belongs there. The TCC surface is the $HOME walk, not the palace store. `trusty-memory setup` and `trusty-memory migrate` call discover_claude_settings, which recurses $HOME to depth 8 for .claude/settings*.json and rewrites what it finds; the skip-list covers Library and Applications but not ~/Desktop, ~/Documents, or ~/Downloads. Every cargo install mints a fresh cdhash and revokes whatever was approved. Its own store under ~/Library/Application Support/trusty-memory/ is NOT the reason — an application's own data directory is not TCC-protected. All three binaries `cargo install --path crates/trusty-memory` produces are signed, applying the #2721 lesson that signing part of an install set leaves the rest ad-hoc and the prompt still recurring. Hardened Runtime takes trusty-search's conservative side of the split rather than trusty-mpm's always-on one: trusty-memory links ort/fastembed through trusty-common's memory-core, so it carries the identical unverified ONNX-dylib-under-library-validation exposure. MEMORY_SET needs no special case in use_hardened_runtime — falling through to plain `explicit` is exactly SEARCH_SET's policy. Two guards that would have caught this class of omission: - every_set_member_has_a_real_identifier iterated the hardcoded array [SEARCH_SET, MPM_SET, AGENTS_SET], so a set added to the table but not to the array was never checked. It now reads the set names out of SIGNABLE_BINARIES itself. - SignTargetArg, not binaries_for_set, is the real gate on `tctl sign` — clap rejects an unlisted value before commands::sign::run runs. A table row without a matching variant is signable in principle and unreachable in practice. every_sign_target_arg_resolves_to_a_real_set now pins both directions. signing_persistence_tip's target list had already gone stale after #4277 (trusty-agents was missing); a test now checks it against the table. 🤖🤖🤖 Generated with trusty-mpm — https://github.com/bobmatnyc/trusty-tools
The per-PR number is what keeps two concurrent PRs from conflicting on the same file (#4476); the fragment was written before the PR existed. 🤖🤖🤖 Generated with trusty-mpm — https://github.com/bobmatnyc/trusty-tools
Verdict: APPROVEAdversarial review at Zero CRITICAL, zero HIGH. Three findings below, all MEDIUM/LOW. Claim verification1. The security rationale — CONFIRMED, independently. Every load-bearing element checks out:
The correction to the original premise is right too — the palace store under 2. Both guard directions — CONFIRMED, by running the breaks.
Both directions are genuinely pinned. See finding 1 for the direction that is not. 3. Hardened-Runtime policy — CONFIRMED, and it is intended rather than accidental.
4. Test honesty — CONFIRMED. I applied all three claimed breaks, plus two of my own.
The third one is the notable result: the new test catches a bug that predates this PR, which is the difference between a guard and a restatement of the diff. 5. Scope restraint — CONFIRMED on both conclusions. The restraint was right; the gap is real. Complete caller set for
Not changing 6. What only a real signing run could establish. See finding 2 — the PR body's hedge is correct and does not propagate into the docs and script it ships. 7. Pre-existing bug — CONFIRMED fixed and in scope. Findings
NotesOn finding 2, the part I could not settle from code. macOS attributes a Files-and-Folders TCC request to the responsible process. For a CLI binary invoked from a shell that is usually the terminal emulator, not the binary — which is precisely the path (
Fail-open check. Clean. Table accuracy. All three binary names verified against Gates I ran myself, in my own worktree: Changelog fragment carries one category ( Not treated as a defect: the #5011 overlap on 🤖🤖🤖 Generated with trusty-mpm — https://github.com/bobmatnyc/trusty-tools |
…tions Owner ruling 2026-08-06 promotes trusty-analyze alongside trusty-memory. It qualifies on the same test one step weaker: `trusty-analyze setup --global` (crates/trusty-analyze/src/commands/setup.rs:129) makes the identical discover_claude_settings walk of $HOME to depth 8, but only behind --global, where trusty-memory walks $HOME on its default setup path. One binary — the crate's only other target is a library. Its Hardened-Runtime placement was confirmed independently rather than inherited by analogy: `cargo tree -p trusty-analyze -i fastembed` shows fastembed as a DIRECT dependency (trusty-memory reaches it transitively via trusty-common's memory-core), so it lands on SEARCH_SET's conservative side for its own reason. hardened_runtime_policy pins both. trusty-review stays out: no $HOME walk, no other-app file access. Review findings: 1. MEDIUM — every_sign_target_arg_resolves_to_a_real_set read as_set_name() but never the clap #[value(name = …)] that actually gates the CLI, so a variant with a typo'd attribute passed while `tctl sign <set>` was rejected. Same vacuity class the test was added to close, one level up. Both strings are now pinned against each other. Proof it works: adding trusty-analyze with a deliberately typo'd clap name goes red with no new test written. 2. MEDIUM — the script and the doc promised the TCC grant would "persist across every future cargo install" while the PR body correctly hedged that nothing was validated by a signing run. Both now state what signing establishes (a stable designated requirement — the precondition) and record that macOS attributes Files-and-Folders access to the responsible process, which for a shell-invoked CLI is usually the terminal rather than the binary. That is exactly the setup/migrate path the rationale rests on, so the CLI-path outcome is unresolved; the launchd daemon path is attributed to the binary and is not affected. 3. LOW — the tip test used substring `contains`, which passes spuriously on any prefix-shaped set name and is blind to a target advertised that is not a set. It now parses the `<a|b>` group and compares exactly, in both directions. 🤖🤖🤖 Generated with trusty-mpm — https://github.com/bobmatnyc/trusty-tools
Verdict: APPROVERe-review at No issues found at >80% confidence. All three round-1 findings are closed, and I verified each by breaking it rather than reading the fix. The Round-1 findings — all three closed, verified by breaking themFinding 1 (guard closed one direction) — CLOSED, and it generalises. The fix pins
The second row is the one I care about. That is byte-for-byte the scenario I built in round 1 that was green on all 559 tests; it is now red with nothing written for it. Finding 2 (doc/script overclaimed) — CLOSED, and the wording survives either attribution outcome. The artifacts now separate what signing establishes from what it achieves: "a stable designated requirement in place of a cdhash that changes on every rebuild. That is the necessary precondition for a durable TCC grant. It is not on its own evidence that one was obtained." That sentence is true whichever way macOS attribution lands. The responsible-process caveat is in both places, and correctly split rather than blanket-hedged — CLI path unresolved, launchd daemon attributed to the binary so a stable identity does apply there. The script's old promise is gone:
Finding 3 (substring Now parses the
Then the decisive one: I kept the prefix break in place and reverted only the test body to the round-1 The
|
Defect
trusty-memoryandtrusty-analyzewere missing fromSIGNABLE_BINARIES(
crates/trusty-installer/src/commands/macos_signing/mod.rs). Owner ruled2026-08-06 on both.
The exclusion was never a decision. I looked before changing anything —
module doc,
SIGNABLE_BINARIEScomments,release-workflow.md,common-pitfalls.md,git log -S"SIGNABLE_BINARIES", and a search for aMEMORY_SETthat had been tried and reverted. Nothing. The table grew onereported symptom at a time — #2558
search, #2721
tm,#2951 the GUI,
#4277
tagent— andnobody filed these two.
Evidence — trusty-memory
The original premise needed correcting on one point.
~/Library/Application Support/trusty-memory/is not the reason — anapplication's own data directory is not TCC-protected, so reading the palace
store prompts for nothing. The real surface is the
$HOMEwalk:trusty-memory setup(crates/trusty-memory/src/commands/setup.rs:415) andtrusty-memory migrate(.../migrate.rs:142) calltrusty_common::claude_config::discover_claude_settings, which recurses$HOMEto depth 8 looking for
.claude/settings*.jsonand rewrites the ones itfinds. Its skip-list (
crates/trusty-common/src/claude_config.rs:30) coverstarget,node_modules,.git,Library,Applications,.Trash,build,dist,.cache,.npm,.cargo— but not~/Desktop,~/Documents, or~/Downloads, so aread_diris attempted on each. Everycargo installmintsa fresh cdhash, so any grant keyed to the old one stops matching.
cargo install --path crates/trusty-memoryproduces THREE binaries and allthree are signed — #2721
is the recorded lesson that signing part of an install set leaves the rest
ad-hoc while the prompt keeps recurring.
Evidence — trusty-analyze (judge this section on its own)
Promoted in the same ruling, added here rather than in a follow-up PR because it
touches the same table, the same clap enum, and the same guard tests as the
round already in flight.
Identical to trusty-memory:
trusty-analyze setup --global(
crates/trusty-analyze/src/commands/setup.rs:129) makes the samediscover_claude_settingswalk of$HOMEto depth 8, reaching~/Desktop,~/Documents, and~/Downloadsthrough the same skip-list gap.Weaker, and the distinction is kept rather than flattened: trusty-memory
walks
$HOMEon its defaultsetuppath. trusty-analyze only does sobehind
--global; project mode patches.mcp.jsonin the project root andnever leaves it. Narrower in how often it is reached, identical when it is.
Hardened Runtime placement was confirmed independently, not inherited by
analogy.
cargo tree -p trusty-analyze -i fastembedshowsfastembedas adirect dependency of
trusty-analyze— trusty-memory reaches ittransitively through
trusty-common'smemory-core. Different edges, sameconclusion: both carry the unverified ONNX-dylib-under-library-validation
exposure, so both fall through to plain
explicitinuse_hardened_runtime,which is exactly
SEARCH_SET's policy.hardened_runtime_policypins eachseparately.
One binary.
cargo metadataconfirmstrusty-analyzehas exactly onebintarget; its only other target is the
trusty_analyzelibrary. There is nobundled sibling to miss the way
tmwas missed in #2721.binaries_for_set_covers_analyzepins that.
trusty-reviewstays excluded, re-verified: nodiscover_claude_settingscall, no
$HOMEwalk, no other-app file access — grepping it for.claude/Desktop/Documents/Downloads/Containers//Volumesreturns onlyBedrock model-name strings (
us.anthropic.claude-sonnet-4-6). Nothing to keep agrant for.
Resolution
MEMORY_SETtrusty-memory,trusty-bm25-daemon,trusty-memory-mcp-bridgescripts/install-trusty-memory-signed.sh/make install-memory-signed/tctl sign trusty-memoryANALYZE_SETtrusty-analyzetctl sign trusty-analyze(no wrapper script — that is the whole surface)SignTargetArg::Memoryand::Analyzeincli.rswere load-bearing, notcosmetic: clap rejects any value the enum does not list before
commands::sign::runis reached, so a table row alone leaves the targetunreachable.
Neither gets an automatic
tctl installpost-install hook, matchingtrusty-agents(#4277) —post_install_search/post_install_mpmremain theonly two.
Review findings
Finding 1 (MEDIUM) — the guard closed one direction and left the mirror open
every_sign_target_arg_resolves_to_a_real_setreadas_set_name()but neverthe clap
#[value(name = …)]that actually gates the CLI. A variant with atypo'd attribute passed all tests while
tctl sign <set>was rejected and thedocs advertised it as valid — the same vacuity class the test was added to
close, one level up. Both strings are now pinned against each other.
Proof it works, and the proof the coordinator asked for: adding
trusty-analyzewith a deliberately typo'd clap name goes red with no newtest written:
Finding 2 (MEDIUM) — script and doc were overclaiming
The PR body hedged that persistence was unconfirmed; the artifacts did not. The
script told the operator signing "makes the approval persist across every future
cargo install" and the doc read as verified guidance. Both now state what signing
establishes — a stable designated requirement, the precondition for a
durable grant — and separate that from an outcome nobody measured.
🔴 Responsible-process attribution, folded into both and flagged here as
unresolved. macOS attributes Files-and-Folders access to the responsible
process, which for a shell-invoked CLI is usually the terminal rather than the
binary — and
setup/migrate(andsetup --global) is exactly that case. Ifthe attribution lands on the terminal, signing these binaries does not change
that path's prompting at all. The launchd-started daemon is attributed to
the binary, so a stable identity applies there. Nobody has confirmed which way
the CLI path resolves, and the doc and script are now written to stay true
either way. I have not tried to resolve macOS semantics here.
Finding 3 (LOW) — substring check tightened
The tip test used
contains, which passes spuriously on any prefix-shaped name(a future
trusty-memset would be "found" insidetrusty-memory) and saidnothing when the tip advertised a target that is not a set. It now parses the
<a|b|c>group and compares exactly, in both directions.signing_persistence_tip's list had already gone stale — it still said<trusty-search|trusty-mpm>after #4277 shippedtrusty-agents.In-flight collision
Two files overlap #5011,
unchanged from the first round:
commands/sign.rs— one doc line and one error-message string.cli.rs— two enum variants and two match arms onSignTargetArg, plusdoc. Not on fix(trusty-installer): stop
tctl upgradedouble-installing daemons, and make its restart real (#4964 phases 0+1) #5011's named surface(
install.rs/upgrade.rs/self_update.rs/lifecycle.rs/bin_resolve) butthe same crate.
macos_signing/, the new script, the Makefile, and the docs are untouched by#5011. Happy to rebase whenever you sequence it.
Gates
cargo fmt --checkcargo clippy -p trusty-installer --all-targets -- -D warningscargo test -p trusty-installer560 passed; 0 failed; 4 ignored(+ 3/3, 3/3 in the integration binaries)cargo check --workspace --exclude trusty-code-gui --exclude trusty-mpm-guibash scripts/check_line_cap.sh3742 files; 7 allowlisted, 0 violations — OKbash scripts/check_sld.sh56 spec docs + 3119 code files; 0 errors, 0 warningsbash scripts/check_test_pointers.sh22072 citations; 0 dangling — OKbash scripts/check_changelog_fragment.shOK trusty-installer: fragment present and validbash -n+shellcheckon the new script--dry-runexercised end to endcargo check --workspace(unfiltered) fails, pre-existing and environmental:trusty-code-guipanics withThe frontendDist configuration is set to "ui/dist" but this path doesn't exist.crates/trusty-code-gui/ui/distis untracked, so itnever exists in a fresh worktree, and
git diff --name-only origin/main...HEAD -- crates/trusty-code-gui/ crates/trusty-mpm-gui/is empty. Excluding those two, the workspace check is clean.
Neither known baseline flake
(
execute_doctor_against_test_daemon,ensure_managed_config_dir_emits_the_frozen_skill_warning,#4931) fired.
Fail-before proof
Every guard was broken at what it names and confirmed red before being restored:
trusty-bm25-daemonrowbinaries_for_set_covers_memory,signable_binaries_table_is_pinned(2 failed, 24 passed)SignTargetArg::Memory→"trusty-memroy"viaas_set_name()every_sign_target_arg_resolves_to_a_real_set: names a set with no binaries#[value(name = "trusty-analzye")]on the NEW variant,as_set_name()intactevery_sign_target_arg_resolves_to_a_real_set— the Finding-1 fix, caught with no new test written<trusty-search|trusty-mpm>signing_persistence_tip_names_every_signable_set— fires on the pre-existing #4277 staleness, not just the new rowsevery_declared_set_is_a_named_constantNot verified
Real
codesignwas not run — no install, sign, orlaunchctlon this machineper the dispatch. Everything asserted here is from the code and the table. Actual
TCC grant persistence across a reinstall, and the responsible-process question
above, both still want manual confirmation on a machine with the Developer ID
certificate.
🤖🤖🤖 Generated with trusty-mpm — https://github.com/bobmatnyc/trusty-tools