Uh oh!
There was an error while loading. Please reload this page.
hyp status reports proxy-mode trust and the launchd env (LLP 0237/0239) - #792
Conversation
LLP 0237's Consequences say `hyp status` reports the keychain trust state alongside the CA fingerprint, and LLP 0239#terminals-predating-attach says it reports whether NODE_USE_SYSTEM_CA is present in the launchd environment. Neither was implemented: `isCaTrusted` was called only from attach and `isLaunchdEnvSet` had zero production callers. Combined with a CA re-mint silently stranding the keychain trust, a user could hold a trust nothing surfaced while every other status line read healthy. The collector now builds a `proxyTrust` section on darwin when a CA is on disk: the fingerprint, `security verify-cert -p ssl`, and `launchctl getenv`. Both probes are injectable and caught independently, so a probe that could not run reports `null` (unknown) rather than a false negative. `--json` carries the tri-state faithfully under `proxy_trust`; the text surface renders the block only where the question applies, with the one-command repair on the untrusted and unset states. Off darwin, and on a darwin host that never turned proxy mode on, the section is absent rather than unknown: both mechanisms are macOS-only (LLP 0237#darwin-only) and no CA means nothing to be trusted or untrusted. Addresses F3 of #790. F1 (the all-policy `add-trusted-cert` grant) and F2 (single-certificate `delete-certificate`) are untouched: both turn on `security(1)` semantics no Linux host can exercise, and F1's fix touches the exact keychain-merge path the maintainer's manual macOS runs proved. Co-Authored-By: Claude <noreply@anthropic.com>
philcunliffe
commented
Aug 15, 2026
VerdictApprove with findings. Nothing here is a blocker. The change does exactly Reviewed manually. Does the surface match what the LLPs claim?Yes, and it stops there.
|
The `--json` comment claimed the V1 shape was "unchanged for an ordinary install". It is not: `proxy_trust` is emitted unconditionally on every platform and is `null` where the question does not apply, which is what this renderer's null-not-omitted contract asks for. Additive and no consumer breaks, but the comment stated the opposite of what the code does. Say what it does instead. README.md and docs/PRIVACY.md still described the pre-proxy CA model, made false by LLP 0237/0238 rather than by this change: - "your system trust store is never touched" / "no other application on the machine is affected". False on macOS since LLP 0237: attach adds the CA to the login keychain as a user-domain trusted root, so any application running as that user which consults the login keychain will accept certificates it signs. PRIVACY.md is where a user makes that decision, so it now says what is installed, why macOS raises the password dialog, and that declining is supported. - "`hyp detach claude` deletes it". False since LLP 0238#ca-survives-detach: detach keeps the CA and the trust so re-attach needs no second dialog. `--purge` and `hyp daemon uninstall` are the removal paths. - "restricted so it cannot vouch for any host other than the one being intercepted". Widened by LLP 0238#full-provider-constraints to the static provider set; named now, since that is the grant the user gives. - README's "`hyp status` shows its fingerprint and expiry": the fingerprint is true as of this PR, expiry is not shown and no LLP asks for it. The claim now matches the surface: fingerprint plus trust state. No LLP was edited; all three are Accepted and already say this. Co-Authored-By: Claude <noreply@anthropic.com>
philcunliffe
commented
Aug 16, 2026
VerdictApprove. Round 2 of 2, the last review round. I reviewed Reviewed manually. Round 1's substantive verifications stand and are not re-litigated: the surface Fixed this round: |
| # | Item | Classification |
|---|---|---|
| 1 | Post-detach standing repair prompt | Preference. Correct about the machine, overall unaffected, ambiguous wording only. Needs mode plumbed into ClientAttachReport plus a behaviour doc. File as follow-up. |
| 2 | hyp status does not name permitted hosts (LLP 0238 "must") | Preference for merging this PR, production-relevant follow-up. It leaves a pre-existing false claim standing rather than creating one, and nothing regresses. But it is the same class of defect as the one this PR exists to fix, on the same surface, and the doc says "must". Should be filed before #790 is closed. |
| 3 | No probe timeout | Preference. Pre-existing, shared helper, wider than F3. |
| - | README / PRIVACY false CA claims | Was a blocker-class doc defect, now fixed in 1f0423c. |
| - | NODE_USE_SYSTEM_CA login-item residue undocumented | Preference. Omission, not a false claim. Fold into whichever change picks up F1/F2. |
| - | Fixes #790 over-closes (author's own flag) | Needs a human, unchanged from the author's note. F1, F2, and finding 2 above all want a narrower macOS-only successor issue. |
Nothing here is a production blocker. Merging is safe.
PR-body text that needs correcting
Two places repeat the claim fixed in 1f0423c. Under Degrading off darwin:
- no CA on disk - proxy mode was never on, so there is nothing to be trusted or untrusted. An ordinary install's text output and JSON shape are unchanged, and the probes are not even called (asserted).
should read
- no CA on disk - proxy mode was never on, so there is nothing to be trusted or untrusted. An ordinary install's text output is unchanged,
proxy_trustisnullrather than absent (the renderer's null-not-omitted contract), and the probes are not even called (asserted).
and in the Coverage line, darwin with no CA (V1 surface unchanged) should
read darwin with no CA (text surface unchanged, proxy_trust null).
The body should also gain a line for the doc corrections in 1f0423c, since
"What this does" currently describes a code-only change: "CorrectsREADME.md and docs/PRIVACY.md, which still described the pre-LLP-0237/0238 CA
model (no trust-store write, detach deletes the CA)."
What I could not exercise
This is a Linux host. Everything about darwin behaviour below is read from code
and the LLP corpus, not run:
security verify-cert/security add-trusted-cert/security delete-certificate, so the realtrustedprobe, the keychain dialog, and a
locked-keychain GUI prompt (finding 3's residual risk) are all unrun.launchctl getenv/setenv/unsetenvand the login LaunchAgent.- The real
hyp attach claudeproxy flow and the post-detachhyp status
render that finding 1 describes. That state is traced throughclient_detach_disk.js,clients.jsand LLP 0238, not observed. - The corrected README / PRIVACY claims are checked against LLP 0237, LLP 0238
and the code that implements them, not against a live keychain. - No
codexand nocode-review, so this is an unassisted manual read.
philcunliffe
commented
Aug 16, 2026
Triage: ship, with successor issue #793Path B. Both review rounds are recorded, the cap is reached, and every unresolved finding classifies as a preference, not a production defect. Gates re-verified independently on this branch at Classification of the unresolved findings1. 2. Post-detach standing repair prompt (round 1, low): preference. Traced independently: 3. No timeout on the probes (round 1, low): preference. Pre-existing property of 4. Docs omit the The |
Uh oh!
There was an error while loading. Please reload this page.
One conflict, in src/core/commands/status.js: both sides added an import line at the same spot. #792 (proxy-mode trust) added `ENV_VAR_NAME` from `../daemon/launchd_env.js`; this branch added `sanitizeLabel` from `../util/json_util.js`. Both kept. The `printable()` collision this PR anticipated did not happen: #681 has not landed, so `src/core/commands/status.js` still holds exactly one `printable()` and one `MAX_ERROR_CHARS` (400), and `src/core/commands/ daemon.js` holds its own single pair. Label sites use `sanitizeLabel`'s 120 default, error sites pass 400. Both intents survive unchanged: this branch's cleaning of every interpolation it targets, including the collector-assembled `remote_config_rolled_back` message whose cleaned prose reaches `--json` while `remote_config.last_rollback` beside it stays byte-exact, and #792's `proxy trust` text block plus tri-state `proxy_trust` JSON key, which stays raw and absent off-darwin exactly as it landed.
Master gained #792, which adds a `proxy trust` section to the same status render and the same collector LLP 0228's maintenance-skip summary lands in. The two are conceptually independent, so every conflict resolved as a union: - src/core/commands/status.js: both imports kept (`describeMaintenanceSkipReasons` and `ENV_VAR_NAME`). - src/core/daemon/status.js: one `@import` line carrying both `MaintenanceSkip*` and `ProxyTrustReport`, plus 0228's cache-types import. - src/core/daemon/status.js: `collectHypAwareStatus` returns both `maintenance` and `proxyTrust`. - src/core/daemon/types.d.ts: `HypAwareStatusReport` carries both fields with their own doc comments. The render bodies merged without conflict and stay in separate places: `proxy trust` after `recent clients`, `maintenance` after `first sync`, each still gated on its own condition, neither swallowing the other's spacing. #792's tri-state (`null` when a probe could not run) and its darwin-plus-CA gate are untouched. LLP 0228 is unique across origin/master, every remote branch, and llp/tombstones after the merge. Co-Authored-By: Claude <noreply@anthropic.com>
Conflict: the import block in `src/core/commands/status.js`. This branch added `sanitizeLabel` (the render-time cleaning for the status-file strings the text surface interpolates); master's #792 added `ENV_VAR_NAME` for the `proxy trust` block's launchd line. Independent, so both imports stay. Everything else auto-merged and was checked rather than assumed. The collector stays raw: `gatewayDroppedUpstreams` still reads `details.upstreams` and `details.upstreams_dropped_names` through `stringList` alone, so `attributeDroppedUpstreams` still intersects unsanitized names with `registered_presets` and its `names.length === dropped` precondition still counts the file's names. `printableUpstreamNames` still derives `+N more` from the raw list's length, and each message's singular/plural still comes off the raw `silent` / `covered` arrays, so cleaning cannot make a plural set read singular. #792's `proxy trust` block, its tri-state `null`-when-unprobed fields, and its absence off darwin or with no CA are untouched. npm test: 4194 tests, 0 fail (baseline origin/master: 4183, 0 fail). npm run typecheck: clean on both. Co-Authored-By: Claude <noreply@anthropic.com>
What this does
Wires the trust and launchd-environment predicates into
hyp status, making two Accepted LLPs' claims true:hyp statusshould report the trust state alongside the CA fingerprint, so 'dialog was cancelled last month' is diagnosable without re-running attach."hyp statusreports whether the variable is present in the launchd environment (launchctl getenv)."Neither was implemented.
isCaTrustedwas called only from attach, andisLaunchdEnvSetwas exported and unit-tested with zero production callers. Combined with the silent CA re-mint the issue describes, a user could trust a root that nothing surfaced, with every other status line reading healthy.The surface
collectHypAwareStatusnow builds aproxyTrustsection (src/core/daemon/status.js,collectProxyTrust):caFingerprintreadLocalCaInfo(already documented as "forhyp status", never called by it)trustedisCaTrusted->security verify-cert -c <ca> -p ssllaunchdEnvSetisLaunchdEnvSet->launchctl getenv NODE_USE_SYSTEM_CAText (
renderStatusText), rendered between the client block and the cache lines:and on the state this exists for:
--jsonstays a faithful machine copy underproxy_trust:{ ca_fingerprint, ca_trusted, launchd_env_set }, ornull.ca_trusted/launchd_env_setare tri-state. A probe that could not run reportsnull, neverfalse: "the dialog was cancelled" and "securitydid not run" are different answers and only the first is actionable, so rounding one to the other would put a false negative in the machine surface. The two probes are caught independently.Both probes are injectable through
CollectStatusOptions(isCaTrusted,isLaunchdEnvSet), matching the existingisLaunchAgentInstalled/launchAgentStatusseams. That is what makes this testable on Linux, and it also keepshyp statusfrom tripping LLP 0181's spawn guard in tests.Degrading off darwin
The section is
null(absent from the text surface entirely) when the question does not apply:proxy_trustisnullrather than absent (the renderer's null-not-omitted contract), and the probes are not even called (asserted).LLP 0225 (strip vs escape)
Deliberately not applicable here, and the code says so: the fingerprint is computed locally from the DER and is
[0-9A-F:]by construction, the other two fields are tri-state booleans, and probe stderr is deliberately not surfaced. No captured or foreign text reaches this render, so there is nothing to strip or escape. No LLP was edited (both are Accepted and already say what this now does).Test evidence (the LLP 0002 gate)
New:
test/core/status-proxy-trust.test.js, six cases drivingcollectHypAwareStatuswith injected probes over a real minted CA in a tempHYP_HOME, then asserting both the rendered text a user sees and the--jsonpayload.Before (
git checkout -- src/on this branch, test file only):Each failure is
+ undefined - null/ a missingproxy trustblock: the collector had no such field and neither renderer printed anything.After:
Coverage: trusted+set (fingerprint on both surfaces), untrusted+unset (repair text present,
overallstillhealthyper LLP 0237#attach-anyway-on-refusal), probes throwing (nullnotfalse, on both surfaces), non-darwin (no section, no "unknown"), darwin with no CA (text surface unchanged,proxy_trustnull), and probes-not-called when there is no CA.Full battery on this branch:
npm test- 4173 pass, 0 fail, 1 skipped (4174 total)npm run typecheck- cleannpm run smoke -- status_diagnostics,walkthrough_picker_to_first_query,client_attach_idempotent,cli_bundled_plugins_activated- all okF1 and F2 remain open, deliberately
Neither is touched, for the same reason: this host is Linux, and no
security(1)invocation can be executed here.-p ssl) is a one-argument change to the exact keychain-merge mechanism the maintainer's live macOS runs A-G proved. Whether Bun's merge honours a policy-scoped trust setting the way it honours an unrestricted one is unverified, and that merge is the whole feature. A blind change risks regressing proven behaviour to harden a same-user-compromise exposure that is not a live hole. It needs a real Mac plus a re-run of the Remote Control acceptance check.delete-certificateremoves one certificate per call, so duplicate-CN roots survive uninstall) depends onsecurity's multi-match semantics, which cannot be exercised here either. Residue, not live risk: re-mint overwritesca-key.pemin place, so a stranded entry vouches for a certificate whose private key is gone.F3's remaining two options from the issue (fingerprint-drift detection with a re-trust prompt; narrowing the re-mint triggers) also stay open. Both change lifecycle behaviour and want a decision doc; this PR is observability only.
Scope note on the closing keyword
Fixes #790below will close the whole issue on merge, including F1 and F2, which this PR does not address. Flagging it explicitly: the recommendation is to re-open a narrower macOS-only issue for F1 + F2 (both need a real Mac and andocs/ACCEPTANCE.mdpass), rather than leave #790 closed as if all three were done. Say the word and the keyword can be downgraded to a plain reference instead.Resolved at triage:
Fixes #790stays. Successor issue #793 now carries F1, F2, F3's remaining lifecycle options, and every finding the #792 review rounds left open (permitted-hosts naming, the post-detach repair prompt, the probe timeout, the launchd-residue doc omission), so nothing is lost when this merges.Documentation corrections
Round 2 also corrected pre-existing drift in
README.mdanddocs/PRIVACY.md, which still described the pre-proxy CA model afterfa701a7landed: they claimed the system trust store is never modified (LLP 0237 installs a login-keychain trusted root), that detach deletes the CA (LLP 0238 keeps it), that name constraints cover only the intercepted host (LLP 0238 widened them), and that status shows fingerprint and expiry (expiry is deliberately not shown). Bounded to the certificate paragraph in each file. No LLP was touched.Fixes#790