Uh oh!
There was an error while loading. Please reload this page.
resolveClientSettingsPath rejects an absolute settings_file instead of re-anchoring it under $HOME - #447
Conversation
…f re-anchoring it under $HOME
An absolute `settings_file` was not an error: `path.join(homeDir,
...settingsFile.split('/'))` swallows the leading empty segment, so
`/Library/Managed Preferences/com.anthropic.claudefordesktop.plist`
silently became `$HOME/Library/Managed Preferences/...`. The env-override
branch was wrong the same way (`parts.slice(1)` assumes a relative first
segment, so it dropped the leading `/` and grafted the rest onto
`$<CLIENT>_HOME`).
The probe then answered about a file the manifest never named. The usual
answer was ENOENT, which reads exactly like a correct "not attached", and
that wrong-negative-indistinguishable-from-a-right-one is how the Claude
Desktop attach_probe defect (#444) stayed invisible. It can also be a
wrong positive: the new probe test builds the marked decoy at the
re-anchored `$HOME` location and the old code reports `attached: true`.
Reject rather than honour: the `$<CLIENT>_HOME` override relocates a
config home, which an absolute path does not have, so honouring would
publish a second silently-different resolution rule for one field. The
resolver is shared by the read side (attach probe, picker detect) and the
write side (the disk-driven undo), so a value core cannot resolve must
fail rather than resolve to something else.
Each caller renders the throw on its own terms: the status probe returns
`{ attached: false, error }` (already carried through to `hyp status`),
`hyp detach` fails loudly, and the init picker keeps its documented
best-effort "not present". Load-time manifest validation is left to
follow-up, sequenced after the Desktop manifest is corrected.
LLP 0045 Part 3 gains the governing section, and the kernel type now
states the home-relative contract it only ever implied.
Co-Authored-By: Claude <noreply@anthropic.com>…under --json The guard makes an unresolvable settings_file an `error` on the probe result, but `renderStatusText` dropped the field: it printed the client as a bare `not attached` and, because such a client is typically not `configured`, usually collapsed the whole section into `clients: (none)`. On stock master + this branch that is exactly what happened to claude-desktop, so the wrong negative the PR set out to end survived one layer up, on the surface a human actually reads. --json already carried it. Print the message under the client's line, and keep an errored client out of the `(none)` collapse. LLP 0045 Part 3 records that both surfaces carry it.
… not the first character
The isAbsolute guard rejects '/etc/passwd' and lets '../../../etc/passwd'
through to exactly the same place. Both land on a file the manifest never
named, which is the whole harm this guard exists to stop, and the escaping
spelling is the one that survives the check:
resolveClientSettingsPath('codex', '../../../etc/passwd', {}, '/Users/hyp')
-> '/etc/passwd' (pre-fix)
.codex/../../../etc/passwd + CODEX_HOME=/tmp/ch -> '/etc/passwd'
It matters more here than in a read-only resolver: this is also the write
side, detachClientFromDisk reads and rewrites whatever it is handed, and
contributes.client is unvalidated, so the value can arrive from a
remotely-installed or org-pushed plugin.
Require the resolved path to stay under the base it resolved against
(settings_file_escapes_base). Each branch checks its own base - $HOME
normally, $<CLIENT>_HOME when set, since the override is exactly a licence
to leave $HOME. A '..' that normalizes away stays legal.
Also drops two overclaims the section had picked up: attach_probe really
does lose expressiveness (it has no absolute sibling, which is why #445
deletes Desktop's probe rather than respelling it), and the one-resolver
argument holds for core but not for the per-plugin attach writers - the
claude plugin hardcodes ~/.claude/settings.json and ignores $CLAUDE_HOME.
And the ClientSettingsPathError JSDoc no longer claims callers avoid
message-text matching when its only consumer flattens to a message.philcunliffe
commented
Jul 29, 2026
Verdict: the approach is right; two actionable findings, both fixed on the branchThe reject-not-honour call is correct, the guard is placed correctly (before both branches), the tests are honest under mutation, and the cross-PR ordering hazard is not real - I traced every caller and exercised the real CLI. Two things were missed:
Head reviewed 1. The ordering hazard: traced per caller, and it does not biteEvery caller of
The two throwing paths both flow through
The all-clients read paths never throw at all, because they go through the probe, which now catches: Exercised against the real CLI on this branch, temp
Worth recording for scale: Also confirmed: claude-desktop's picker row uses 2. (a) reject vs (b) honour: reject is right, and the "already in force" claim checks outBoth stated reasons survive independent scrutiny:
The "documents a rule already in force" claim is true, verified against
So it enforces a stated-but-unenforced contract. Deferring manifest-load validation is right and is recorded with its sequencing. Two things the section overclaimed, now corrected in
3. Test quality: verified by mutation, not by readingReverted only the behaviour, keeping the Both failures behavioural, not module-load, exactly as claimed. Test 5's
4. FindingsMEDIUM - fixed in |
settings_file | resolved to |
|---|---|
../../../etc/passwd | /etc/passwd |
./../../etc/passwd | /etc/passwd |
.codex/../../../etc/passwd (+ CODEX_HOME=/tmp/ch) | /etc/passwd |
/Library/Managed Preferences/x.plist | THREW |
Every word of the PR's own justification - "silently probes a file the manifest never named", "a wrong negative indistinguishable from a right one" - applies verbatim to ../../../etc/passwd, which escapes $HOME just as completely and is the spelling that survives the check. It matters more here than in a read-only resolver: detachClientFromDiskreads and rewrites the resolved path, and contributes.client is unvalidated anywhere (src/core/manifest.js has no client-side checks; src/core/plugin_catalog.js:77 copies client.attach_probe verbatim), so the value can arrive from a hyp plugin install <git url> or an org-pushed plugins[] entry.
Fixed by enforcing the contract on the resolved path: it must stay under the base it resolved against (code: 'settings_file_escapes_base'), each branch checked against its own base ($HOME normally, $<CLIENT>_HOME when set, since the override is precisely a licence to leave $HOME). A .. that normalizes away (.codex/sub/../config.toml) stays legal. Verified after the fix: all four escapes throw, and .claude/settings.json, .codex/config.toml (both branches), .openclaw/openclaw.json, .claude-desktop/settings.json + CLAUDE_DESKTOP_HOME all resolve exactly as before. Mutation-tested: disabling the containment check fails the new test alone.
LOW - the typed code is dropped at its only consumer (JSDoc corrected, 623eb6b)
The class doc said it is typed "so a caller can turn it into whatever observable means ... without matching on message text", but status.js:971 does error: err.message and the probe result type is error?: string, so the tests then match on message text - exactly what the doc says the type avoids. Adding error_code to the probe result would change the public hyp status --json shape, which a review should not do unilaterally, so I corrected the JSDoc to claim only what is true (the code is there for callers that need to branch; callers that only need observability forward the message). Worth a follow-up if you want error_code on the JSON surface.
INFO - a reconciler reverse for such a manifest now retries forever
action_attach.jsreverse() -> detachClientFromDisk now throws -> {status:'failed'} -> the reconciler keeps the marker and retries every pass. On master it resolved to a bogus $HOME path, no-op'd changed:false, and cleared the marker. The new behaviour is the documented intent ("core must not claim to have reversed"), but it is an unbounded retry + error-log loop that the LLP does not mention. Not reachable for claude-desktop (it registers no ctx.clients adapter, so desired() is inert for it), so this needs a third-party plugin with both an absolute settings_file and a live adapter. Left as-is; flagging for the record.
INFO - path.isAbsolute is correct for the paths in play; POSIX-only is fine
Platform-dependent, and both behaviours are the wanted one: on POSIX and Windows alike '/Library/...' -> true, '.codex/config.toml' -> false. A Windows-style C:\... would slip on POSIX, but the resolver's whole model is POSIX (settingsFile.split('/')), the daemon installer supports launchd and systemd only (src/core/daemon/platform.js), and no manifest is Windows-shaped - and it now lands inside$HOME as a weird filename rather than anywhere dangerous. Not worth a UNC/drive-letter branch.
INFO - @ref anchors resolve; house style clean
#### \settings_file` is home-relative, and a violation is loudslugifies tosettings_file-is-home-relative-and-a-violation-is-loud - underscores kept, backticks and comma stripped - matching repo precedent in the same file (#enable_tool_search-keep-deferred-tool-loading-on-through-the-gatewayfrom#### ENABLE_TOOL_SEARCH: keep deferred tool loading on through the gateway, and #conflict--back-up--override-restore-on-leave). The heading is unique, so no -1suffix. All refs well-formed and attached without a blank-line break. No em dash on any added line, no semicolons, no inlineimport('...')types, no@typedef. npm run typecheck` clean.
Separately: .claude/skills/ref-check/SKILL.md:80 describes slugification as "non-alphanumerics stripped", which read literally would strip the underscore and reject both this anchor and the pre-existing #enable_tool_search-... one. Pre-existing doc bug in the skill, not this PR's; worth a one-line fix elsewhere.
5. Verification of the fixes
npm test on 623eb6b: 2799 pass / 8 fail, the same 8 leave-command.test.js failures as origin/master. npm run typecheck clean. npm run smoke -- core_boot_noop and client_attach_idempotent pass. (claude_attach_detach and client_attach_on_join fail identically on origin/master in this environment - pre-existing, unrelated.) Both new guards mutation-tested: disabling either check fails its own test and nothing else.
Merge-ordering guidance (please read when sequencing)
#447 is safe to merge in either order relative to #445. It does NOT have to wait.
- resolveClientSettingsPath rejects an absolute settings_file instead of re-anchoring it under $HOME #447 before claude-desktop: drop the attach_probe core cannot read or reverse #445 (the interim state):
hyp statusshowsclaude-desktop [not in config, not attached]plus the error line on both surfaces, exit 0, for every install (the descriptor is present even though the plugin is excluded from default activation).hyp detach --client alldetaches every other client normally and exits 1 with one per-client error.hyp detach(no--client),hyp attach,hyp init,hyp join,hyp leave, and the login attach-wait are all unaffected. That is hyp detach --client claude-desktop always fails with MALFORMED_MARKER #444's defect becoming visible, which is the point. - claude-desktop: drop the attach_probe core cannot read or reverse #445 before resolveClientSettingsPath rejects an absolute settings_file instead of re-anchoring it under $HOME #447: no shipped manifest declares an absolute
settings_file, so resolveClientSettingsPath rejects an absolute settings_file instead of re-anchoring it under $HOME #447 becomes a pure guard against future ones plus the traversal containment (which is worth having on its own merits, independent of Desktop). - Trial merges: no conflicts against any of the four held heads. detach: report every externally-overridden managed key, not just the last (issue #440 finding 2) #441 touches
client_detach_disk.js, which resolveClientSettingsPath rejects an absolute settings_file instead of re-anchoring it under $HOME #447 does not; claude-desktop: drop the attach_probe core cannot read or reverse #445 touches only the claude-desktop manifest and LLPs. - The one mild preference: land claude-desktop: drop the attach_probe core cannot read or reverse #445 first. It avoids the interim
hyp detach --client allexit 1 and thehyp statuserror row for users who never enabled Claude Desktop. Both are honest signals rather than breakage, so if resolveClientSettingsPath rejects an absolute settings_file instead of re-anchoring it under $HOME #447 lands first nothing is wrong - but claude-desktop: drop the attach_probe core cannot read or reverse #445 -> resolveClientSettingsPath rejects an absolute settings_file instead of re-anchoring it under $HOME #447 is the tidier sequence, and claude-desktop: drop the attach_probe core cannot read or reverse #445 is already approved.
Reviewed at 607925e; fixes pushed as 9403fba and 623eb6b.
…prefix test Round-2 review of the containment guard added in 623eb6b. The guard holds against every escape spelling I could build (absolute, `..` in either branch, prefix-sharing siblings, `..` landing on the base), but three things about it were either untested or unwritten: - The `base + separator` suffix in the prefix test is the entire check - a bare `startsWith(base)` accepts `/home/username` under a `/home/u` base - and deleting the suffix made no test fail. Pinned. - `withinBase` validated `path.resolve(joined)` and returned the raw `joined`. They diverge whenever the base is relative: `CODEX_HOME=..` returned the relative `../config.toml`, i.e. a value re-resolved against `process.cwd()` at read time rather than the one validated at call time, and not the absolute path the resolver's contract promises. Return the checked path. - The check is lexical, not realpath-based, so a config home that is itself a symlink out of $HOME still passes. That is the right boundary - the field is resolved before the file must exist, and planting such a symlink already needs write access to $HOME, whereas the untrusted input is the unvalidated manifest value - but it was nowhere stated, and an undocumented limit on a security control invites over-trust. Written down in the JSDoc and in LLP 0045 Part 3. Co-Authored-By: Claude <noreply@anthropic.com>
…o it The errored `claude-desktop` row was pushed onto whatever `collectHypAwareStatus` had already produced, and the bundled catalog produces a `claude-desktop` row of its own carrying this very error - the manifest defect #445 removes. So the list held two same-named rows and the JSON assertion's `find` answered from the catalog's, passing for the wrong reason. It stops passing the moment #445 lands: the catalog row loses its error, `find` still returns it first, and `the JSON renderer carries the same client error` fails. Verified both ways - `git merge origin/fix/issue-444` into this branch failed that test before this commit and passes after, with the 8 known leave-command failures unchanged in both trees. Replace the same-named row rather than joining it. These tests are about the two renderers, so they own the whole client list and assert nothing about which clients the bundled catalog happens to ship. Co-Authored-By: Claude <noreply@anthropic.com>
philcunliffe
commented
Jul 29, 2026
Round 2 (final, cap 2) — reviewed |
| Vector | Result |
|---|---|
/etc/passwd | settings_file_absolute |
//etc/passwd (double-slash spelling) | settings_file_absolute |
/ | settings_file_absolute |
../../../etc/passwd | settings_file_escapes_base |
./../../etc/passwd | settings_file_escapes_base |
.. (lands on the parent) | settings_file_escapes_base |
../../homework/x | settings_file_escapes_base |
../username/x against base /home/u (prefix-sharing sibling) | settings_file_escapes_base |
../hyp2/x against base /Users/hyp | settings_file_escapes_base |
../../etc/passwd with base /Users/hyp/ (trailing slash) | settings_file_escapes_base |
.codex/../../../etc/passwd + CODEX_HOME=/tmp/ch | settings_file_escapes_base |
../../../etc/passwd + CODEX_HOME=/tmp/ch | settings_file_escapes_base |
a/../../../etc/passwd + CODEX_HOME=/tmp/ch | settings_file_escapes_base |
../outside/secret.toml with a symlinked base | settings_file_escapes_base |
The path.relative/startsWith pitfall is not present. The check
(client_settings_path.js:100) istarget !== root && !target.startsWith(root + path.sep) on both sidespath.resolved. The + path.sep is what makes /home/username fail against a/home/u base, and it is there. Trailing slashes on the base are normalized
away by path.resolve first (/Users/hyp/ and /Users/hyp// both behave as/Users/hyp), so a trailing-slash base is not a hole either.
Not refused, correctly (documented as such after this round)
- Symlink escape. A
settings_fileof.codex/secret.tomlwhere$HOME/.codexis a symlink to/outsideresolves lexically inside the base
and is accepted;realpathof the result is/outside/secret.toml. The
guard does not resolve symlinks, and it should not: the field is resolved
before the file has to exist (attach creates it, the picker only stats its
directory), sorealpathwould fail on exactly the paths that matter most —
and planting that symlink already requires write access to$HOME, at which
point the settings file is the attacker's regardless. The untrusted input
here is the manifest value, and that is what the guard contains. This was
nowhere written down; an undocumented limit on a security control invites
over-trust, so it is now stated in the JSDoc and in LLP 0045 Part 3. ..landing exactly ON the base.'','.',.codex/..all return the
base itself;target === rootis explicitly allowed. Benign and loud: the
callers thenreadFilea directory,EISDIRis notENOENT, soprobeClientAttachFromDescriptor's outer catch (status.js:1025-1032)
returns{ attached: false, settingsPath, error }. Not a silent wrong
answer, so not the class of bug this PR exists to kill. Same for the
override branch's single-segment degenerate case (config.toml+CODEX_HOME=/tmp/chyields/tmp/ch, the directory) — pre-existing, loud.- A hostile
$<CLIENT>_HOME.CODEX_HOME=/etcwith.codex/passwd
returns/etc/passwdand passes, because the override is checked against
itself. By design and correct:$<CLIENT>_HOMEis the user's own
environment, not the untrusted manifest, and the override is precisely a
licence to leave$HOME. The guard still contains the manifest value
within the override, which is the part that matters. - NUL / newline in the path. Pass the guard (they are legal path
characters lexically) and are rejected downstream byfswithERR_INVALID_ARG_VALUE, which is notENOENTand so surfaces as anerror
on the probe result. Loud, not silent. - Backslash and
C:\...spellings. Treated as literal single segments on
POSIX and contained. The repo is POSIX-targeted (systemd units,~/.claude),
so not a gap. - A hostile client name (
'../codex'). Used only for the env-key
derivation (already sanitized byreplace(/[^A-Z0-9]/g, '_')) and the error
text, never for the path. No effect.
2. Test teeth — verified by mutation
Each guard was mutated in place and the named test confirmed to fail. This also
re-establishes round 1's pre-fix-failure claims behaviourally: reverting only
the behaviour (M1, M3) reproduces the failures the PR body reports.
| Mutation | Test that fails |
|---|---|
M1 withinBase containment neutered | rejects a settings_file that climbs out of its base |
M3 path.isAbsolute guard removed | rejects an absolute settings_file rather than re-anchoring it, probeClientAttachFromDescriptor errors on an absolute settings_file…, the JSON renderer carries the same client error |
M4 override branch checked against homeDir (wrong base) | rejects a settings_file that climbs out of its base |
M5 text renderer drops the error: line | the text renderer prints a client probe error instead of a bare not-attached |
M6 (none) collapse ignores c.error again | both text-surface tests |
M2 startsWith(root) — path.sep deleted | nothing (before this round) |
M7 return joined instead of return target | nothing (before this round) |
M2 and M7 are findings, below. Both now fail against named tests.
Full suite on 623eb6b: 2799 pass / 8 fail — exactly the knowntest/core/leave-command.test.js baseline. npm run typecheck clean. Same
baseline on 9fdb606 (2801 pass, +2 new tests, 8 fail).
3. Findings
MEDIUM — the tests break the moment #445 lands (cross-PR, fixed)
test/core/status-client-error.test.js:48-58 built its fixture by pushing
an errored claude-desktop row onto whatever collectHypAwareStatus returned.
The bundled catalog already produces a claude-desktop row carrying this exact
error (hypaware.plugin.json:23, the defect #445 removes), so the list held
two same-named rows and the JSON assertion's rows.find(r => r.name === …)
answered from the catalog's row. It passed for the wrong reason.
Once #445 lands the catalog row loses its error, find still returns it first,
and the test fails. Demonstrated, not inferred:
git merge origin/fix/issue-444 # into 623eb6b+
npm test -> 9 failures: the 8 leave-command baseline
+ not ok - the JSON renderer carries the same client error
actual: undefined
Fixed by replacing the same-named row instead of joining it. Re-verified: the
merged tree now runs 2804 pass / 8 fail, the baseline exactly.
This is also why the sibling-prefix and check-vs-return findings below matter —
all three are "the test passes but not for the reason it claims".
LOW (security-hygiene) — the + path.sep had no test (fixed)
Deleting the separator from client_settings_path.js:100 — the single most
common way a containment check is silently wrong — broke no test (M2). The
implementation is correct; nothing was pinning it. AddedresolveClientSettingsPath does not mistake a prefix-sharing sibling for the base, covering /home/u vs /home/username, /Users/hyp vs /Users/hyp2,/home/hyp vs /home/hypaware, plus the override branch. Confirmed it fails
under M2.
LOW — the guard checked one string and returned another (fixed)
withinBase validated path.resolve(joined) but returned the raw joined.
They diverge whenever the base is relative: with CODEX_HOME=.. the function
returned the relative../config.toml, i.e. a value re-resolved againstprocess.cwd() at read time rather than the one validated at call time — and
not the absolute path the resolver's own contract promises ("Resolve the
absolute settings-file path"). Not exploitable (both sides resolve against the
same cwd within a call, and $<CLIENT>_HOME is the user's env), but a
check/use divergence introduced by this PR's own guard is worth closing while
it is one line. Now returns target; pinned byresolveClientSettingsPath returns the absolute path it checked, confirmed
failing under M7. No behaviour change for absolute bases (path.resolve is
identity on path.join's output there) — full suite unchanged.
LOW (doc) — the symlink limitation was unstated (fixed)
See the attack section. Recorded in the JSDoc and LLP 0045 Part 3 rather than
changed, with the reasoning for why lexical is the right boundary.
Confirmed sound, no action
- Round 1's three findings all land as described in
623eb6b. - "
hyp detachfails loudly" — verified end to end rather than assumed:detachClientViaCore's dry-run path (clients.js:422) anddetachClientFromDisk(client_detach_disk.js:89) both let the throw
propagate, andbin/hypaware.js:53-58catches it intohyp: <message>with
exit 1. Clean message, no stack trace. - All four callers enumerated in the PR body are the only ones.
grep resolveClientSettingsPathalso findsplugins-workspace/openclaw/src/settings.js:83, which passes a hardcoded
constant — unaffected, and the PR body's own "one honest caveat" paragraph
already covers the per-plugin writers. @refanchors resolve. All eight point atllp/0045-client-attach.design.md:287,#### \settings_file` is
home-relative, and a violation is loud`; the slug matches.- House style. No semicolons, no em dashes on any added line (the two the
scanner sees are pre-existing context in LLP 0045), no@typedef, no inlineimport('...')types, JSDoc types throughout.npm run typecheckclean.
Nothing left for triage.
4. Merge ordering — round 1's conclusion needs one amendment
Round 1 concluded #447 is safe in either order relative to #445
(fix/issue-444), preferring #445 first. On 623eb6b that was wrong: the
combined tree failed a test (finding 1). On 9fdb606 it is right, and now
demonstrated in both directions rather than argued:
- resolveClientSettingsPath rejects an absolute settings_file instead of re-anchoring it under $HOME #447 alone on master: 2801 pass / 8 baseline fail.
hyp statusgrows an
error row forclaude-desktop, which is hyp detach --client claude-desktop always fails with MALFORMED_MARKER #444's defect becoming visible,
the PR's stated intent. - resolveClientSettingsPath rejects an absolute settings_file instead of re-anchoring it under $HOME #447 merged with claude-desktop: drop the attach_probe core cannot read or reverse #445: 2804 pass / 8 baseline fail. The interim error row
disappears because no shipped manifest declares an absolutesettings_file. git merge-tree --write-treeagainst all four held heads (Attach declares the gateway first-party so Claude keeps its real context window #438fix/issue-437, Give the session opt-out a reader: hyp session ignore/unignore/status, fail-closed #439fix/issue-432, detach: report every externally-overridden managed key, not just the last (issue #440 finding 2) #441fix/issue-440, claude-desktop: drop the attach_probe core cannot read or reverse #445fix/issue-444): clean from9fdb606.
Guidance: either order is safe. Mild preference for #445 first — it avoids
shipping a hyp status that shows an error row for a bundled plugin, and round
1's text-surface fix makes that row more visible than it was at 607925e, so
the cosmetic argument for #445-first is slightly stronger now, not weaker. No
code or test dependency either way.
What changed this round
623eb6b → 9fdb606 (2 commits, +114/-13 across 4 files):
b3351a1—withinBasereturns the path it checked; symlink boundary
documented in JSDoc + LLP 0045; two new resolver tests (prefix-sharing
sibling, absolute-return).9fdb606—status-client-error.test.jsowns its client list instead of
appending to the catalog's.
No host state touched: no hyp subcommand of any kind was run, no~/.claude/settings.json, no ~/.codex/config.toml, no ~/.config/systemd/.
All work in a detached worktree; every attack ran as a direct function call or
against a mkdtemp fixture.
philcunliffe
commented
Jul 29, 2026
Triage: PR #447 safe to ship (draft, ready to un-draft)Classification: PREFERENCE only. No true blocker found. Round 2's verdict Independent verification of the merge-ordering claimRound 2's crux claim is that its own round-1 merge-ordering conclusion was
Conclusion: the merged-tree result is real, reproducible, and not an Lexical (non-realpath) containment guard - judged independentlyRead On lexical-vs-realpath: agree this is the right boundary given the threat Body correctionThe body's "Ground truth" section still quoted Merge-ordering guidanceRound 2's amended position stands and I confirm it independently: either Follow-up issueNone filed. Both review rounds found and fixed everything they raised in-round No host state touched: no |
Uh oh!
There was an error while loading. Please reload this page.
The fork, and which way it went
The issue offers two coherent fixes: reject an absolute
settings_file, orhonour it. This PR rejects, per the issue's own leaning, for three reasons
that come out of the code rather than taste:
$<CLIENT>_HOMEexists to relocate a client's config home, which is thefirst segment of a home-relative path. An absolute path has no config home,
so honouring would have to publish a second, silently-different resolution
rule for one field.
resolveClientSettingsPathbacks the attach probe and the picker'ssettings_filedetect and the disk-driven undo (LLP 0045 Part 3). A valuecore cannot resolve must fail, or attach and detach can disagree about which
file they own.
the only shipped manifest that declared an absolute
settings_fileis ClaudeDesktop's, and that declaration is the defect hyp detach --client claude-desktop always fails with MALFORMED_MARKER #444/claude-desktop: drop the attach_probe core cannot read or reverse #445 is removing.
This does not change a published contract that needs ratification. The
home-relative rule was already the stated contract (it lived in
resolveClientSettingsPath's JSDoc); it was simply never enforced.hypaware-plugin-kernel-types.d.tshad no doc comment onsettings_fileatall, so this PR writes down the rule that was already in force rather than
introducing a new one.
The silent part is the bug
An absolute
settings_filewas not an error.path.join(homeDir, ...settingsFile.split('/'))swallows the leading empty segment, so/Library/Managed Preferences/com.anthropic.claudefordesktop.plistquietlybecame
$HOME/Library/Managed Preferences/.... The env-override branch waswrong the same way:
parts.slice(1)assumes a relative first segment, so itdropped the leading
/and grafted the remainder onto$<CLIENT>_HOME. Bothbranches are settled by one guard before either runs.
The probe then reported on a file the manifest never named. Usually ENOENT,
which
probeClientAttachFromDescriptormaps to a bareattached: falsewith noerrorfield: a wrong negative indistinguishable from a right one, which isexactly how the Desktop
attach_probedefect stayed invisible. It can also be awrong positive, which the new probe test demonstrates directly.
So the probe now returns
{ attached: false, error }for an unresolvablesettings_file.src/core/commands/status.jsalready forwards a clientreport's
errorintohyp status --json, so no rendering change was needed.Every caller of
resolveClientSettingsPath, and how it behaves nowsrc/core/daemon/status.jsprobeClientAttachFromDescriptorattached: false(or a wrongtrue){ attached: false, error }, surfaced byhyp statussrc/core/commands/clients.jsdetachClientViaCore(dry-run path)$HOME-re-anchoredsettings_pathsrc/core/config/client_detach_disk.jsdetachClientFromDisksrc/core/cli/detect.jsprobeIsPresentNone relies on the re-anchoring. The picker's absolute-literal needs are
already served by the sibling
app_bundleandpathdetect variants (LLP0136), so no manifest loses expressiveness.
Ground truth
Two new tests in
test/core/daemon.test.js, both proven failing on the pre-fixbehaviour and passing after (verified by reverting only the behaviour, keeping
the new export, so the failure is behavioural and not a module-load error):
resolveClientSettingsPath rejects an absolute settings_file rather than re-anchoring it- covers both branches: no override, andCLAUDE_DESKTOP_HOMEset. Pre-fix:Missing expected exception(it returned/Users/hyp/Library/...and/tmp/claude-desktop-home/Library/...). Assertsthe typed
ClientSettingsPathErrorwithcode: 'settings_file_absolute'.probeClientAttachFromDescriptor errors on an absolute settings_file instead of probing $HOME- plants a marked decoy at the re-anchored$HOMElocation, so the pre-fix code reports
attached: trueagainst a file themanifest never named. Pre-fix:
true !== false. Assertsattached: false, nosettingsPath, noversion, and anerrornaming the violation.npm test: 2794 pass, 8 fail - exactly the known pre-existingtest/core/leave-command.test.jsbaseline, unchanged by this PR.npm run typecheck: clean.(Corrected post-review: the review rounds added more resolver and renderer
tests after this line was written. At the merged head
9fdb606this PR aloneruns 2801 pass / 8 fail, still exactly the
leave-command.test.jsbaseline;npm run typecheckremains clean. Independently re-verified during triage.)What is deliberately NOT in this PR
Rejecting at manifest load would be better (the plugin author learns at
install, not at probe time), and the issue suggests it. It is left to follow-up
because
contributes.clientis not validated at all today, and the bundled@hypaware/claude-desktopmanifest onmasterstill declares the absolutepath. Adding the check now would take a shipped plugin out of the catalog to
punish a defect already being fixed in #445. LLP 0045 records the deferral and
its sequencing.
Sequencing against held PRs
Based on
origin/master. Trialgit merge-tree --write-treeagainst all fourheld heads (#438
fix/issue-437, #439fix/issue-432, #441fix/issue-440,#445
fix/issue-444): clean, no conflicts.src/core/config/client_detach_disk.js. This PR does nottouch that file - the throw propagates through it unchanged, which is the
intended loud failure.
claude-desktop: drop the attach_probe core cannot read or reverse #445 removes the
attach_probeblock entirely, so once it lands no shippedmanifest declares an absolute
settings_fileand this change is a no-op forbundled plugins. Before it lands, on
masterwith this PR alone,hyp statusshowsclaude-desktopwithattached: falseand anerrorinstead of a silent false - which is hyp detach --client claude-desktop always fails with MALFORMED_MARKER #444's defect becoming visible rather
than masked. That is the intended interim state, not a regression.
Docs
llp/0045-client-attach.design.mdPart 3 gains#### settings_file is home-relative, and a violation is loud, landed in thesame commit as the code, and the three new
@refs point at it.Fixes#446