feat(server): list open sessions on the landing page - #1
Closed
flpdorea wants to merge 4 commits into
Closed
Conversation
visibleSessions() in cli.js filtered out ended sessions with a private one-liner. The upcoming server-side landing list needs the exact same filter, and a second copy of `status !== "ended"` is exactly the kind of thing that quietly drifts between the two surfaces over time. Export it once from session-store.js so the CLI's home output and the server's landing page can never silently diverge on what "open" means.
GET / always rendered the same static "Lavish Editor is running" card,
regardless of how many sessions were actually open - useful only if you
already have the CLI to hand. On a phone via the tailnet root there is
no CLI at all, and even on desktop it's convenient to just open the
root URL and see what's live.
The route now reads the same session inventory the CLI already prints
(filterVisibleSessions() over store.listSessions()) and renders a
minimal read-only list when any are open: a status dot, the file path
with the home directory elided the same way the chrome overflow menu
already does (displayPathParts), and a link. The empty state is
byte-identical to today's placeholder.
Decisions worth recording:
- Read failures fail open to the placeholder rather than 500ing - this
route is unauthenticated and always reachable, so it must never hang
or error on a bad state.json read.
- Each link is built fresh as a relative /session/{key} at render time,
never the session's stored `url` field. That field is captured once
at POST /api/sessions time against whatever host resolved then, and
would go stale relative to whichever host actually served this page
once the tailnet root (kunchenguid#216) exists alongside plain loopback.
- No opt-out env var. This is a deliberate new-default change, accepted
as such, so no extra config surface was added for it.
- The populated list carries a 15s <meta http-equiv=refresh> so it
stays current without a new SSE surface; the empty placeholder never
carries it, since there is nothing there to go stale and reloading it
would disrupt someone leaving the tab open for later.
- session.file is a filesystem path an attacker could shape (this route
has no auth beyond the Host allowlist), so every user-controlled
string is escaped with the existing escapeHtml().
Only one existing test touched the landing route at all (the Tailscale dual-listener test), and it only ever saw the empty placeholder. Extend it to assert the session it already opens shows up in the list with a working relative link resolved against the MagicDNS host - proving tailnet-root compatibility rather than merely asserting it. New coverage: - empty state stays byte-identical to today's placeholder, no refresh tag - a single open session renders file name, status dot, working link, and the refresh tag - several sessions list correctly and an ended one is excluded - a file path containing markup-significant characters is escaped, not rendered raw - this route has no auth in front of it - filterVisibleSessions as a pure unit test: keeps open/feedback, drops ended, preserves order
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.
What Changed
/landing route now reads visible sessions from the store and renders each as a row (status dot, truncated file path, "Open" link) instead of the static placeholder card; the empty-list case is unchanged.filterVisibleSessionsinsrc/session-store.js(dropsstatus === "ended"sessions) and reused it from bothsrc/cli.js'svisibleSessions()and the new server route, so the CLI's home output and the landing page can't diverge on what counts as "open".<meta http-equiv="refresh">tag, but only when the session list is non-empty, so an idle empty page never reloads out from under a viewer.state.jsonread error falls back to an empty session list instead of a 500, since/is unauthenticated and must stay available.Risk Assessment
✅ Low: This fix round only added one accurate documentation line to README.md with no functional or source code changes; the prior security finding was already reviewed and explicitly accepted by the user in round 1.
Testing
Installed missing dependencies (chokidar etc. via
pnpm install, which the worktree lacked) then ran the three targeted test groups covering this change end-to-end: the landing page correctly renders the empty-state placeholder with no refresh tag when no sessions are open, lists open sessions with escaped file paths/status dots/working session links and a 15s refresh tag when populated, excludes ended sessions while keeping order, HTML-escapes a maliciously crafted file path, and the session link resolves correctly cross-host under simulated Tailscale binding (validating the relative-link design decision). The extractedfilterVisibleSessionshelper is also unit-tested directly. All 7 relevant tests passed; no code or test changes were needed, and the working tree is clean (node_modules/dist installed for testing are gitignored, not tracked).Pipeline
Updates from git push no-mistakes
⏭️ **intent** - skipped
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed ✅
src/server.js:530- The new/route is unauthenticated (protected only by the Host allowlist, not by session-key possession) and now lists every open session's real filesystem path plus a one-click/session/:keylink. Visiting that link immediately callsissueReviewerHandoff, which mints a fresh reviewer token and supersedes whatever browser tab currently holds the review - with no confirmation step. Previously an attacker/curious user needed to already know (or receive) the exact artifact path to compute or be given the session key; now anyone who can reach the bound interface (loopback-sharing users, or any device on a Tailscale-bound tailnet per the documented non-loopback warning) can enumerate every open review and hijack any of them in one click, with no opt-out env var. The commit message documents this as a deliberate accepted default, but that acceptance appears to have been made unilaterally rather than confirmed with the project owner, and this materially lowers the bar for reviewer-session takeover within the existing 'unauthenticated server, trust the network' threat model.README.md- The landing page session list is a new user-facing feature (file paths, status dots, refresh behavior, session links) but README.md's user-facing contract was not updated to describe it, contrary to AGENTS.md's documentation-ownership rule that README owns features and their behavior.🔧 Fix: docs(readme): document new landing page session list
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
node --test --test-name-pattern "landing" test/server.test.js(5 tests: empty placeholder, single session listing, multiple sessions excluding ended, HTML-escaping of malicious file paths, health/landing responsiveness)node --test --test-name-pattern "Tailscale mode binds" test/server.test.js(verifies the session link on the landing page is relative/host-independent and resolves correctly when fetched via the MagicDNS host, per issue #216)node --test --test-name-pattern "filterVisibleSessions" test/session-store.test.js(unit test for the extracted shared filter used by both the CLI home output and the new landing page)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.