Skip to content

hyp status names the CA's permitted hosts, uninstall clears every duplicate root (#793) - #800

Merged
philcunliffe merged 6 commits into
masterfrom
fix/issue-793
Aug 19, 2026
Merged

hyp status names the CA's permitted hosts, uninstall clears every duplicate root (#793)#800
philcunliffe merged 6 commits into
masterfrom
fix/issue-793

Conversation

@philcunliffe

@philcunliffe philcunliffe commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Fixes three of the findings on #793 and the doc omission it names. Every change was written and proven on Linux; the two macOS-only findings that need a real Mac (F1, F3 residue) are left open and called out at the bottom.

hyp status does not name the CA's permitted hosts (medium) - fixed

LLP 0238 Consequences: "The dialog and hyp status must name all permitted hosts, so the grant is informed." The dialog half was implemented; the status half was not. collectProxyTrust had the whole LocalCaInfo in hand, including hosts, and returned only the fingerprint.

  • ProxyTrustReport gains hosts: string[], filled from the certificate's own permitted dNSName subtrees, so the line is the grant itself and cannot drift from config.
  • Text surface gains permitted: api.anthropic.com, api.openai.com, chatgpt.com inside the existing proxy trust: block; --json gains permitted_hosts.
  • A certificate with no dNSName constraint at all renders as "not host-limited" rather than as a blank line, since that reading is the one that matters.

Proof (test/core/status-proxy-trust.test.js): the new test every permitted host the trust grant covers is named on both surfaces mints a CA over the full static provider set on an install that captures one of them, and asserts all three hosts on both surfaces. Before the fix: not ok 1, not ok 2 (# pass 5 # fail 2). After: # pass 7 # fail 0.

removeCaTrust deletes one certificate per call (F2, low) - fixed

Every HypAware CA carries the same CN, so a re-minted machine holds several indistinguishable trusted roots and uninstall cleared exactly one. The survivors outlive the key they were minted with and nothing looks for them again.

  • Removal now sweeps in a bounded loop (8 passes) until security reports no match left, which is also why "could not be found" already had to read as the end state rather than a failure.
  • Hitting the bound is reported rather than silently accepted, and a real failure (locked keychain, denied authorization) stops the sweep and hands the detail back.
  • purgeProxyTrustResidue now prints a detail alongside a successful removal: "some went" and "something is left" are no longer exclusive.

Proof (test/core/tls-darwin-trust.test.js): removeCaTrust clears every identically named root, not just the first drives a fake keychain holding three identically named certificates and asserts none survives. Two further tests cover the failure stop and the bound. Before the fix: not ok 6, not ok 7, not ok 8 (# pass 6 # fail 3) - the three-duplicate keychain still held two roots. After: # pass 9 # fail 0.

Caveat carried from the issue: security's multi-match semantics are still unverified on a real Mac. The loop is safe under either reading - if one invocation already deleted all matches, the sweep costs exactly one extra call that reports not-found - but a docs/ACCEPTANCE.md pass on a Mac with a re-minted CA is the confirmation.

No timeout on the trust probes (low, pre-existing) - fixed

runServiceCommand spawned with no timeout, and hyp status on darwin now shells out through it twice. The residual risk named on the issue is a locked login keychain raising a GUI prompt.

  • runServiceCommand(bin, args, { timeoutMs }) - opt-in, because the commands reaching this seam are not alike: a mutation the user is answering a password dialog for may take minutes, a read-only status probe may not. On expiry the child is SIGKILLed and the promise rejects with a new ServiceCommandTimeoutError.
  • Rejecting rather than returning non-zero is the load-bearing part: isCaTrusted maps exit codes to a boolean, so a killed process would otherwise render as "not trusted - run hyp attach claude", a false negative dressed as a measurement. As a rejection it lands on the existing catch and renders unknown - the keychain probe could not run.
  • isCaTrusted and isLaunchdEnvSet accept timeoutMs; hyp status passes 5s. The interactive attach path is untouched and still waits unbounded.

Proof (test/core/service-manager-test-sandbox.test.js): runServiceCommand kills a command that outlives its timeout runs a 120s child with timeoutMs: 250 in a child process (the guard's opt-in is process-global, following the existing opt-in test's pattern). Before the fix: not ok 7 (the helper takes no options, so the wait never ends and the outer 20s bound trips). After: ok 7. A companion test pins that an unset timeout still waits.

Docs omit the launchd residue (low, omission) - fixed, non-behavioural

Neither README.md nor docs/PRIVACY.md mentioned that macOS proxy attach sets NODE_USE_SYSTEM_CA=1 via launchctl setenv and installs a login LaunchAgent to re-apply it. Both now state it, with the plist path, that it is a login item, that a running terminal app must be fully quit to see the variable, and that detach / --purge / uninstall remove it. Every claim is read off src/core/daemon/launchd_env.js (installLaunchdEnv, removeLaunchdEnv, envAgentPlistPath under defaultPlistDir), src/core/config/client_detach_disk.js (releaseProxyModeLaunchdEnv) and src/core/commands/clients.js (purgeProxyTrustResidue). Both files also now mention that hyp status names the permitted hosts, which is true as of this PR.

Still open

  • F1 (medium): the keychain grant is all-policy. Adding -p ssl touches the exact keychain-merge path the live macOS runs proved, and whether Bun's merge honours a policy-scoped trust setting is unverified. Needs a Mac and a Remote Control acceptance re-run, so it is not attempted here.
  • F3 residue: fingerprint-drift detection and narrower re-mint triggers. Both change lifecycle behaviour and want a decision doc, not a repair.
  • Post-detach repair prompt (low). Gating the launchd env: line on "a proxy attach is live" needs mode plumbed out of the attach probe into ClientAttachReport plus a decision about what hyp status claims in that state (the same gate is exactly right for "CA minted by the daemon, never attached"). A design change; left for a doc.

Checks

  • npm test: 4222 pass, 0 fail.
  • npm run typecheck: clean.
  • npm run smoke -- status_diagnostics and npm run smoke -- client_attach_idempotent: ok.
  • No acceptance-tier verification was possible: this ran on Linux, and the touched paths that only execute on darwin (the sweep, the probe timeout in situ) still want a docs/ACCEPTANCE.md pass on a real Mac before a release that ships them.

Fixes #793

test and others added 2 commits August 17, 2026 20:48
…licate root (#793)

Three findings from #793, plus the doc omission it names.

- `hyp status` reported the CA fingerprint but not the hosts the grant
  covers, which LLP 0238's Consequences require ("the dialog and `hyp
  status` must name all permitted hosts, so the grant is informed"). The
  set is read off the certificate's own permitted subtrees, so it cannot
  drift from what the keychain actually vouches for.
- `removeCaTrust` deleted one certificate per call while every HypAware CA
  carries the same common name, so a re-minted machine kept trusting the
  older roots after an uninstall that reported success. Removal now sweeps
  in a bounded loop until the keychain reports no match left, and says so
  if it hits the bound.
- `runServiceCommand` had no timeout, and `hyp status` on darwin shells out
  through it; a locked login keychain can put `security` behind a GUI
  prompt with nobody watching. The helper takes an opt-in `timeoutMs`, the
  two read-only probes accept one, and status sets it. A timed-out probe
  rejects rather than returning non-zero, so it renders as `unknown` and
  never as a false "not trusted".
- README and docs/PRIVACY.md now state the macOS launchd residue: attach
  runs `launchctl setenv NODE_USE_SYSTEM_CA 1` and installs a login
  LaunchAgent that re-applies it.

F1 (`-p ssl` on the trust grant) and the F3 lifecycle residue stay open:
both need a real Mac and an acceptance pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… the LaunchAgent runs

The permitted-host line is the one field of the proxy-trust report that is
bytes off disk: a dNSName is an IA5String read out of whatever certificate
sits at the CA path and decoded as latin1, with no charset or length check
on the way. Our own mint refuses a non-printable host, but the renderer's
'not host-limited' arm exists precisely because a foreign certificate there
is reachable, so a crafted one could carry an ESC run or a newline into a
line hyp status prints. Sanitized at collection, like every other label in
that file, and the JSDoc that claimed no LLP 0225 sanitizing applied here is
corrected rather than left to mislead the next reader.

PRIVACY.md said the login LaunchAgent 'starts no process of its own'. It
execs /bin/launchctl once per login (buildEnvAgentPlist), which is what the
code comment says and what a privacy document should say too.
@philcunliffe

Copy link
Copy Markdown
Contributor Author

Verdict

Approve with two fixes pushed. The three behavioural changes are sound and I could not break any of the three load-bearing claims the PR body makes. Two findings, both low, both fixed on this branch (c9ed499f); three nits left deliberately, with reasons. Two things genuinely need a real-Mac acceptance run and are called out at the bottom.

Reviewed at head db37d2b3, in a clean worktree off origin/fix/issue-793. npm test 4222/0 before my changes, 4223/0 after; npm run typecheck clean; npm run smoke -- status_diagnostics ok. No em dashes in the diff, no stray semicolons in code, @ref anchors all resolve (0238#consequences, 0238#ca-survives-detach, 0238#full-provider-constraints).


What I verified, rather than took on trust

The 8-pass sweep is safe (src/core/tls/darwin_trust.js:128-152). It cannot spin: the loop's only continue is exitCode === 0, and the bound is a hard for limit. It cannot stop early silently: every exit arm is either the confirmed not-found end state (removed, no detail) or carries a detail, and the clients.js:1603-1611 change is what makes that detail reach the user even alongside removed: true. It cannot delete more than a single call already could: the argv is byte-identical on every pass and the new test asserts that per call. Bound exhaustion is reported, not swallowed (stopped after 8 passes; ... may remain - remove them in Keychain Access).

Rejecting on timeout really is load-bearing (src/core/daemon/service_ops.js:49-60,160-186). Confirmed the mechanism end to end: a SIGKILLed child closes with code === null, which the pre-existing close handler maps to exitCode: -1; isCaTrusted is return result.exitCode === 0 (darwin_trust.js:68), so that would have resolved false, and describeCaTrust (src/core/commands/status.js:~570) renders false as not trusted - run \hyp attach claude`. A false negative dressed as a measurement, exactly as the PR body says. As a rejection it lands on collectProxyTrust's catch { trusted = null }and rendersunknown - the keychain probe could not run`.

No leaked child and no double-settle: SIGKILL on expiry, clearTimeout on both error and close, and the timedOut guard suppresses the post-kill close. timer.unref() cannot cause a missed timeout, because a live child with piped stdio keeps the loop referenced. The default really stays unbounded - opts = {} means no timer is created at all - and both the interactive attach callers (installCaTrust, and claude/src/index.js:536) pass none.

permitted_hosts cannot drift from config. collectProxyTrust takes ca.hosts from readLocalCaInfo (src/core/tls/ca.js:369) → permittedHosts(cert)readNameConstraints(cert.raw).permittedDns (src/core/tls/x509.js:461), i.e. parsed straight out of the certificate's DER permittedSubtrees. No config value reaches that path. Nothing sensitive is newly printed: provider hostnames off a static list.

Every README/PRIVACY claim, checked against the cited code. launchctl setenv NODE_USE_SYSTEM_CA 1 at attach → installLaunchdEnv. Plist path → envAgentPlistPath = defaultPlistDir (platform.js:48-50, ~/Library/LaunchAgents) + ENV_AGENT_LABEL + .plist, matching the documented string exactly. "re-runs that one command at each login" → buildEnvAgentPlist: RunAtLoad true, no KeepAlive, ProgramArguments = /bin/launchctl setenv NODE_USE_SYSTEM_CA 1. "already-running terminal app must be fully quit" → matches installLaunchdEnv's own doc and LLP 0239#terminals-predating-attach. Detach unsets and removes → releaseProxyModeLaunchdEnv (client_detach_disk.js:713), gated on marker.mode === 'proxy' + darwin, which is the right gate for the paragraph's context. --purge and hyp daemon uninstallpurgeProxyTrustResidue calls removeLaunchdEnv unconditionally on darwin and is reached from both (clients.js:150, clients.js:1444). All correct except one, below.


Findings

1. hyp status printed unbounded certificate bytes to the terminal, and the JSDoc said it did not - low, fixed

src/core/daemon/status.js:1202-1205 (pre-fix) asserted: "Nothing here carries text from another process onto the terminal - the fingerprint is computed locally from the DER and is [0-9A-F:] by construction, and probe stderr is deliberately not surfaced - so no LLP 0225 sanitizing applies." This PR adds a second field to the same report that breaks that claim.

A dNSName is an IA5String, and readNameConstraints decodes it as der.subarray(base.start, base.end).toString('latin1') (src/core/tls/x509.js:496) with no charset check and no length bound. describePermittedHosts then join(', ')s it straight onto stdout (src/core/commands/status.js:437). IA5 includes ESC, so a certificate at the CA path carrying evil[2Kforged.example in a permitted subtree repaints the line of whoever runs hyp status.

Reachability is narrow, which is why this is low and not medium: our own mint refuses it outright (assertAsciiHost, x509.js:239-249), so it takes a hand-crafted PEM written to <stateRoot>/tls/ca-cert.pem - an attacker who already has the state root. But the renderer's own not host-limited arm exists because a foreign certificate there is treated as reachable, and a status label carrying bytes we did not write is precisely the case sanitizeLabel is the repo's policy for; this same file already applies it to entrypoint and client_name at status.js:416-422.

Fixed by sanitizing at collection (one entry in, one entry out - a host that sanitizes away is named (unprintable dNSName) rather than dropped, since dropping it would understate the very grant this field exists to state), correcting the JSDoc, and noting it on ProxyTrustReport.hosts and describePermittedHosts.

New test a permitted host carrying terminal control bytes cannot repaint hyp status mints a CA with a same-width filler host, byte-patches the DER (equal length keeps every ASN.1 length valid; nothing on the read path verifies the signature), and asserts the ESC and the newline are gone from both surfaces. Verified as a real guard: with the sanitize line reverted it is not ok 3 (# pass 7 # fail 1); with it, ok 3 (# pass 8 # fail 0).

2. PRIVACY.md says the LaunchAgent "starts no process of its own" - low, fixed

docs/PRIVACY.md:69. It does start one: buildEnvAgentPlist sets ProgramArguments to /bin/launchctl setenv … with RunAtLoad, so macOS execs /bin/launchctl once at every login. The code comment right above it is precise ("the program runs once per login, sets the variable, and exits. The agent is inert configuration, not a resident process"); the doc's paraphrase drops the distinction between no resident process and no process. In a privacy document a user reads to decide what is on their machine, that is the wrong direction to round. Reworded to name what actually runs and to keep the (true) "nothing is sent anywhere" claim.

3. Exactly 8 duplicates produce a false residue warning - nit, left

src/core/tls/darwin_trust.js:135-152. Eight successful deletes consume all eight passes, the loop falls out, and the caller prints ! keychain trust may not be fully removed (stopped after 8 passes…) on a keychain it just cleared completely - sending the user to Keychain Access for nothing. Inherent to any bound, hedged by "may", and it takes eight CA re-mints to reach. Fixing it costs a confirming ninth pass and churns the bound test, so I left it; worth a line in the doc comment if anyone touches this again.

4. The two trust probes run serially, so hyp status can block for 2 x 5s - nit, left

src/core/daemon/status.js:1250,1258. Bounded, darwin-with-CA only, and Promise.all would halve it - but it would also flatten the independent per-probe catch that the surrounding comment argues for at some length ("a probe that could not run reports null, never false"). Not worth the trade in this PR.

5. hyp detach --purge --json drops the purge lines - pre-existing, out of scope

src/core/commands/clients.js:151 prints purged.lines only under if (!parsed.json). So the new ! keychain trust may not be fully removed residue warning - the whole point of the clients.js change here - is invisible to a --json caller. Pre-existing shape, not introduced by this PR, and fixing it means deciding where those lines belong in the JSON envelope. Flagging it so it does not get lost.


Needs a real Mac (acceptance tier, not guessable from Linux)

  • security delete-certificate -c multi-match semantics. The sweep is correct under either reading, and its failure arm is safe if security errors on an ambiguous match rather than deleting one - but which it does is still unverified. A docs/ACCEPTANCE.md pass on a Mac with a re-minted (ideally 2-3x re-minted) CA, asserting Keychain Access holds no HypAware Local CA after hyp daemon uninstall, is the confirmation.
  • The 5s probe timeout in situ. The unit test proves the helper kills and rejects; it does not prove that a locked login keychain puts security verify-cert behind a GUI prompt that the SIGKILL actually clears, nor that 5s is the right budget for a cold keychain. Lock the login keychain, run hyp status, and confirm the line reads unknown - the keychain probe could not run with no dialog left on screen.
  • F1 (all-policy grant) and the F3 lifecycle residue remain correctly deferred, for the reasons the PR body gives.

New head after my two fixes: c9ed499fc731b7acdbffe2075b3dbfc4f0ef4bbb.

… surface can be flooded by them

`hyp status` was taught to strip control bytes out of the certificate's own
dNSName subtrees before printing them. The line the attach adapter writes
immediately before macOS raises its password dialog names the same bytes from
the same `readLocalCaInfo` read, and was not, so an ESC run or a newline in a
subtree could repaint the sentence a user grants trust on the strength of.

Neither surface bounded the *count* either, which is the third way one of these
values is hostile and the one `recentEntrypointsFromSources` already answers a
few hundred lines above the status collector.

Both are now one shared policy, `displayableCaHosts`, next to the read that
produces the bytes. It strips (label plane), names an entry that sanitizes away
rather than dropping it, and ends a list longer than any real CA's with a count
of what it left out - because both callers exist to state how wide a trust
grant is, and a silently shortened list understates exactly that.

LLP 0225#scope leaves `hyp status` and the client attach adapters to make the
strip-versus-escape argument for themselves; this is that argument.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
Contributor Author

Verdict

Approve with one fix pushed. Round 2 of 2, reviewed at head c9ed499f in a clean worktree off origin/fix/issue-793. Round 1's two fixes are correct; one of them was incomplete, and completing it is the only new finding. The three nits round 1 left are all still real and all still correctly left, for the reasons below. Nothing in the parts round 1 declared sound came apart under a second look.

npm test 4223/0 before my change, 4228/0 after (+5 tests); npm run typecheck clean; npm run smoke -- status_diagnostics and npm run smoke -- client_attach_idempotent ok. No em dashes in the diff, no stray semicolons, 0225#scope resolves.


New finding

1. The sanitize fix stopped one surface short, and left the third hostile dimension open on both - low, fixed

Round 1 fixed hyp status. It did not fix the other surface that prints the same bytes from the same read, and it answered two of the three ways such a value is hostile.

a) The attach dialog was still unsanitized. hypaware-core/plugins-workspace/claude/src/index.js:543 (pre-fix):

`  Requesting keychain trust for the HypAware Local CA (limited to: ${hosts.join(', ')}).\n`

Those hosts are ca.hosts from readLocalCaInfo (index.js:230) - byte-for-byte the same source collectProxyTrust reads, decoded latin1 out of the DER by readNameConstraints (src/core/tls/x509.js:496) with no charset check. Same reachability as round 1's finding (a certificate written to <stateRoot>/tls/ca-cert.pem), so the same low severity - but arguably the worse of the two lines to leave open, because it is written immediately before macOS raises the trust password dialog. An ESC run or a newline there repaints the sentence the user is granting trust on the strength of, and can forge a narrower, more reassuring host set than the one actually being granted. Round 1's own justification ("a status label carrying bytes we did not write is precisely the case sanitizeLabel is the repo's policy for") applies to this line at least as strongly.

Worth noting the corpus is explicit about this: LLP 0225#scope names hyp status and "the client attach adapters" together as label-plane surfaces that each need "its own argument about strip-versus-escape". Fixing one and not the other left half of that argument made.

b) Neither surface bounded the count. readNameConstraints (src/core/tls/x509.js:461-509) pushes one entry per permitted subtree with no cap, and round 1's fix mapped over all of them (src/core/daemon/status.js:1267, pre-fix). sanitizeLabel bounds each entry's length, not how many there are. This file already answers that dimension a few hundred lines up, and its comment enumerates exactly the three: "control and invisible bytes (sanitizeLabel), unbounded length (sanitizeLabel), and unbounded count" (status.js:393-397, with MAX_RECENT_ENTRYPOINTS at status.js:375,430). A hand-crafted CA with tens of thousands of subtrees floods a hyp status line and the --json payload.

Fixed by lifting the policy into one shared exported helper next to the read that produces the bytes - displayableCaHosts in src/core/tls/ca.js:386-420 - and routing both surfaces through it (status.js:1260, claude/src/index.js:550). It strips (label plane, per 0225#scope), names an entry that sanitizes away as (unprintable dNSName) rather than dropping it, and ends an over-long list with (+N more dNSName constraints). Both callers exist to state how wide a trust grant is, so nothing may vanish silently in either direction: a stripped entry is still an entry, a clipped entry keeps sanitizeLabel's ellipsis, and a truncated list says how much it withheld. ProxyTrustReport.hosts JSDoc updated to match (src/core/daemon/types.d.ts:254-260).

On round 2's explicit question - does (unprintable dNSName) understate the grant? No, and it is the right call over dropping. Dropping would silently shrink the stated grant by exactly one subtree, which is the one direction this field must never round in. The remaining imprecision is that the identity of that subtree is unrecoverable from the line - but it is unrecoverable by construction (the bytes are unprintable), and hyp status is not the tool for reading a certificate; openssl x509 -text is. With (b) fixed, all three hostile dimensions are now stated rather than hidden.

Verified as real guards, not decoration. Five new tests in test/core/tls-ca.test.js. Mutating displayableCaHosts to the identity function turns them not ok 15, not ok 16, not ok 17, not ok 18 (# pass 14 # fail 4) and also fails round 1's a permitted host carrying terminal control bytes cannot repaint hyp status (status-proxy-trust.test.js, # pass 7 # fail 1), which is the positive proof that both surfaces now hang off the one policy. Restored: # pass 26 # fail 0 across both files.

Also swept up while in docs/PRIVACY.md: round 1's reworded paragraph left `hyp detach\nclaude` broken across a line mid-code-span (PRIVACY.md:72-73, pre-fix). Renders fine, reads badly; rewrapped. Non-behavioural.


Round 1's three open nits - all re-examined, all still left

2. Exactly 8 duplicates produce a false residue warning - nit, left (agreeing with round 1)

src/core/tls/darwin_trust.js:136-153. Confirmed the mechanism: the loop's only exit that clears the warning is the not-found pass, so 8 successful deletes consume the bound and fall out with detail set. I looked for a fix and there is not one worth having. The obvious shape - allow MAX + 1 calls where the last is only a confirmation - does not remove the off-by-one, it moves it to 9, and it churns the bound test's calls.length assertion for no change in behaviour at any count a machine reaches. Any bound has this edge; the message hedges with "may"; reaching it takes eight CA re-mints. Round 1's call stands.

3. The two trust probes run serially, so hyp status can block 2 x 5s - nit, left

src/core/daemon/status.js:1247,1255. Round 1 declined because Promise.all would flatten the independent per-probe catch. That specific objection is soluble - starting both promises with their own catch before awaiting keeps the tri-state exactly - but the trade still is not worth it here. The worst case is a locked login keychain, where 10s versus 5s is not what the user is going to notice, and the block being restructured is the one carrying the argument for why each probe is caught alone. Left with the mechanics noted for whoever next touches it.

4. hyp detach --purge --json drops the purge lines - pre-existing, left

src/core/commands/clients.js:151 still gates purged.lines on if (!parsed.json), so the new ! keychain trust may not be fully removed residue warning is invisible to a scripted caller. Confirmed pre-existing and confirmed not made worse by this PR - it is the same gate on the same array. Fixing it is a JSON envelope decision (a purge block? warnings folded into the existing warnings key?), which is a design question and not a repair. Re-flagging so it survives this PR.


Re-checked from round 1, still sound

  • The sweep. Re-derived independently: the only continue is exitCode === 0, the bound is a hard for limit, the argv is byte-identical on every pass (asserted per call by the new test), every non-zero exit either returns the not-found end state or returns with a detail, and bound exhaustion returns a detail rather than swallowing. clients.js:1602-1612 is what makes a detail alongside removed: true reach the user.
  • The timeout rejection. runServiceCommand's new defaultRunner replacements are exactly equivalent when no bound is passed: both launchd_env.js:38 and darwin_trust.js:36 are (cmd, args) => runServiceCommand(cmd, args), and runServiceCommand creates no timer at all when timeoutMs is undefined (service_ops.js:165). So isCaTrusted / isLaunchdEnvSet called without a bound behave byte-for-byte as before, and only hyp status opts in (status.js:1116-1118). No double-settle (the timedOut guard suppresses the post-SIGKILL close), no leaked timer (clearTimeout on both error and close).
  • permitted_hosts cannot drift from config. Re-walked readLocalCaInfopermittedHosts(cert)readNameConstraints(cert.raw).permittedDns. No config value reaches it. Separately confirmed the other consumers of ca.hosts are safe without sanitizing, so this PR's surface really was the whole exposure: createLeafStore (ca.js:310) and the gateway's ca_permitted_hosts log (ai-gateway/src/source.js:500) both take LocalCa from ensureLocalCa, and loadLocalCa only reuses a stored certificate whose permitted set matches the requested hosts exactly (ca.js:243-246), so those strings are ours by construction.
  • README / PRIVACY claims. Spot-re-checked the launchd half against buildEnvAgentPlist and envAgentPlistPath; still accurate, and round 1's "starts no process of its own" correction reads right.

One residual nit, not worth a change: a dNSName containing ", " renders as two entries in the text surface's joined line. It cannot understate the grant (it can only appear to widen it), --json is unambiguous, and it is unreachable through our own mint.


Needs a real Mac (unchanged from round 1, still owed)

  • security delete-certificate -c multi-match semantics. The sweep is correct under either reading; which reading is true is still unverified. A docs/ACCEPTANCE.md pass on a Mac with a 2-3x re-minted CA, asserting Keychain Access holds no HypAware Local CA after hyp daemon uninstall.
  • The 5s probe timeout in situ. The unit test proves the helper kills and rejects. It cannot prove that a locked login keychain's GUI prompt is actually cleared by the SIGKILL, nor that 5s is right for a cold keychain. Lock the login keychain, run hyp status, confirm unknown - the keychain probe could not run and no dialog left on screen.
  • The attach dialog line I changed is macOS-only and could not be exercised here: ensureDarwinProxyTrust is gated on process.platform === 'darwin' and is not exported, so my proof is at the shared-helper level (unit tests on displayableCaHosts, plus the wiring under tsc). The rendered line itself wants an eyeball on the same acceptance run - it should read (limited to: api.anthropic.com, api.openai.com, chatgpt.com), exactly as before for any real CA.
  • F1 (all-policy grant) and the F3 lifecycle residue remain correctly deferred.

New head after my fix: 121f316f28d26ba6500c0871888355f4db1463c1.

@philcunliffe

Copy link
Copy Markdown
Contributor Author

Triage at head b8a4033d54997bd59e014cb1f462a1c2b427a45e (round-2 head 121f316f plus a trivial merge of green master; remerge-diff empty, so no PR content changed after round 2). Every residual finding from both review rounds was re-verified against this head and classified non-blocking: the exactly-8-duplicates false residue warning, the serial 2x5s trust probes, the detach --purge --json line drop (pre-existing, wants a JSON-envelope decision), and the ", "-in-dNSName rendering nit. All of them, plus the still-open design items from #793 (F1 all-policy grant, F3 lifecycle residue, post-detach repair prompt) and the three macOS acceptance items (delete-certificate multi-match semantics, the 5s probe timeout against a locked keychain, the rendered attach-dialog line), are tracked in #829 so the Fixes #793 close loses nothing. Verified here: npm test 4269 pass / 0 fail, npm run typecheck clean, npm run smoke -- status_diagnostics ok.

Two conflicts, both from #818 (LLP 0262) moving Claude Code off proxy
capture onto OTEL telemetry attach.

hypaware-core/plugins-workspace/claude/src/index.js: resolved to master.
This branch's only change there sanitized the permitted-host list in
`ensureDarwinProxyTrust`'s keychain dialog, and #818 deleted both that
function and its call site - `hyp attach claude` no longer mints or trusts
a CA, so the dialog it hardened does not exist. The shared half of that
work survives: `displayableCaHosts` still lives in src/core/tls/ca.js and
`hyp status` still routes the hosts through it.

README.md: kept master's client-generic rewrite of the proxy-mode section
and folded this branch's two additions back into it. `hyp status` naming
"every host the CA is permitted to vouch for" is still true and still this
branch's change (LLP 0238 Consequences, extended but not superseded by
0262, and master's collectProxyTrust still returned no hosts). The launchd
residue bullet is kept but re-scoped to `<client>` and to the attach that
trusted the CA, since no attach path installs the variable now; the removal
paths it documents (detach, --purge, uninstall, the #818 migration unwind)
all still exist, as does the `launchd env:` status line.

docs/PRIVACY.md, src/core/daemon/status.js and src/core/daemon/types.d.ts
merged cleanly; the new proxy-trust `hosts` field does not collide with
#818's `client_telemetry_stale` diagnostic or #777's status.json cleanup.
@philcunliffe

Copy link
Copy Markdown
Contributor Author

Deferred non-blocking review findings to #829. Triage at head 306210b.

This head is the round-2 head 121f316 plus two merges of green master. The second merge (ecdfd82) resolved two conflicts with #818 (LLP 0262, Claude Code moved from proxy capture to OTEL telemetry attach): hypaware-core/plugins-workspace/claude/src/index.js resolved to master, deleting the attach dialog whose host line round 2 sanitized, because the dialog itself no longer exists; README.md folded this branch's two additions into master's client-generic proxy section. Verified here that the resolutions are sound: displayableCaHosts still guards the one remaining host-printing surface (src/core/daemon/status.js:1615), and the two readLocalCaInfo call sites #818 added in the gateway log only certPath and static strings, never certificate host bytes.

Residual findings, all re-verified at this head and all non-blocking:

  • exactly-8-duplicates false residue warning (src/core/tls/darwin_trust.js:151): cosmetic edge of any bound, hedged, takes eight re-mints to reach
  • serial 2x5s trust probes in hyp status (src/core/daemon/status.js:1467-1469): bounded latency, darwin-with-CA only
  • detach --purge --json drops the purge lines (src/core/commands/clients.js:151): pre-existing, wants a JSON-envelope decision
  • ", "-in-dNSName rendering nit: cannot understate the grant, --json unambiguous
  • new comment-only nit from the merge: src/core/daemon/status.js:1574 still names the deleted attach dialog

The deferred design items (F1 all-policy grant, F3 lifecycle residue, post-detach repair prompt) and the macOS acceptance items remain tracked in #829; the attach-dialog acceptance item is now moot and is noted there.

Verified at this head: npm test 4495 pass / 0 fail, npm run typecheck clean, npm run smoke -- status_diagnostics ok.

@philcunliffe philcunliffe added the neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) label Aug 19, 2026
@philcunliffe
philcunliffe merged commit f7780ec into master Aug 19, 2026
9 checks passed
@philcunliffe
philcunliffe deleted the fix/issue-793 branch August 19, 2026 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Follow-up: F1 and F2 from #790, plus proxy-trust status findings deferred from PR #792

1 participant