fix(server): always show remote URL + close share-html symlink gap#929
Merged
Conversation
Two pre-release fixes found by the QA sweep.
1. Remote URL stranding: handleServerReady only opened a browser (silent on
a headless box) and wrote a ready-file; the user-visible URL came solely
from writeRemoteShareLink, gated on sharingEnabled. So a remote/SSH user
with sharing disabled (PLANNOTATOR_SHARE=disabled or config.json
{share:"disabled"}, widened by #921) saw no URL and the agent hung on
waitForDecision() forever. Now handleServerReady prints the reachable
localhost URL whenever the session is remote, independent of sharing; the
share link stays an extra. Pi/OpenCode already printed the URL
unconditionally and are unaffected.
2. share-html symlink gap: the #927 symlink-containment fix hardened the
/api/html-assets asset sinks but missed packages/server/annotate.ts's
/api/share-html containment, which stayed lexical. A symlinked *.html
inside the doc directory pointing outside it leaked the target's contents
into the share payload. Now realpath-resolves both root and target like
the other sinks. Bun-only — Pi's copy was already hardened.
Adds regression tests for both (handleServerReady remote stderr; share-html
returns 403 on symlink escape).
… sinks The symlink-containment check existed as four byte-identical copies (Bun html-assets route, share inliner, annotate /api/share-html, Pi server). That duplication is exactly why the escape was missed in one sink before — #927 hardened three and missed the fourth, and the #929 fix added a fifth-in-waiting. Export the single canonical isWithinDirectory from the shared (Pi-vendored) html-assets-node module and have every sink import it; delete the three duplicate bodies and their now-dead realpathSync/relative/isAbsolute imports. A new sink can no longer silently diverge. Regression tests + 690 server/shared tests pass; build:pi clean.
…emote Remote Pi users never saw a review/annotate/last URL: it was emitted from openBrowserForServer AFTER the command's turn ended (fire-and-forget), and Pi only renders a notify during an active turn — so it silently dropped. The 'X opened. You can keep chatting' notice fires in-turn and DOES show, so fold the URL into it for remote sessions (single-line, matching the notify convention). Local sessions are unchanged (browser auto-opens).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes the session URL reliably visible to remote users across runtimes, closes a symlink-containment gap, and de-duplicates the containment helper. Surfaced by the pre-release QA sweep + real Pi user reports.
1. 🔴 Remote URL visibility — Claude Code hook
On a headless remote box the browser can't open, and the hook's only user-visible URL came from
writeRemoteShareLink, gated onsharingEnabled. With sharing disabled (PLANNOTATOR_SHARE=disabledorconfig.json {"share":"disabled"}, widened by #921) the user saw no URL and the agent hung onwaitForDecision().handleServerReadynow prints the reachablelocalhost:<port>whenever remote, independent of sharing.2. 🔴 Remote URL visibility — Pi (review / annotate / last)
Remote Pi users never saw a URL for these slash commands. Root cause: the URL was emitted from
openBrowserForServerafter the command's turn ended (fire-and-forget), and Pi only renders anotifyduring an active turn — so it silently dropped. (The "X opened. You can keep chatting" notice fires in-turn and shows fine.) Fix: fold the URL into that in-turn message for remote sessions, single-line per the notify convention. Local sessions unchanged.3. 🟠
/api/share-htmlsymlink containment (completes #927)#927 hardened the
/api/html-assetsasset sinks withrealpathSyncbut missedannotate.ts's/api/share-htmlcontainment, which stayed lexical — a symlinked*.htmlin the doc dir leaked its target. Now realpath-resolved. Bun-only (Pi was already hardened).4. ♻️ Consolidate the containment helper
The four byte-identical
isWithinDirectorycopies (the reason a sink kept getting missed) are now one exported function inpackages/shared/html-assets-node.ts, imported by all sinks (Bun + the Pi vendored copy). A new sink can't silently diverge.Tests / verification
shared-handlers.test.ts: remote session prints URL; local doesn't.annotate.test.ts:/api/share-htmlreturns 403 (no leak) on symlink escape.packages/server+packages/shared: 690 pass, 0 fail. typecheck clean;build:piclean; hook binary rebuilt.Known follow-ups (non-blocking)
!readyFile; not yet included./api/docroute (handleDoc) uses the same lexical containment as the share-html sink did — a likely pre-existing symlink class, out of scope here, worth a separate follow-up.