control-plane: add session logs to dashboard#53
Conversation
- GET /logs — recent session log entries (JSON), filterable by session - GET /sessions — list recent sessions with metadata - Dashboard now has Status and Logs tabs - Logs tab shows recent agent activity: user prompts, agent responses, tool calls - Auto-refreshes every 30s
Greptile SummaryAdds session log viewing to the control plane dashboard: two new JSON endpoints (
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Browser
participant Dashboard as GET /dashboard?tab=logs
participant LogsAPI as GET /logs
participant SessionsAPI as GET /sessions
participant FS as Session JSONL Files
Browser->>Dashboard: Request logs tab
Dashboard->>FS: findSessionFiles()
FS-->>Dashboard: Sorted JSONL file list
Dashboard->>FS: tailSessionFile() per file (up to 10)
FS-->>Dashboard: Raw JSONL entries
Dashboard->>Dashboard: summarizeEntry() + filter
Dashboard-->>Browser: Server-rendered HTML with log entries
Browser->>LogsAPI: GET /logs?session=X&lines=50
LogsAPI->>FS: findSessionFiles()
FS-->>LogsAPI: Filtered file list
LogsAPI->>FS: tailSessionFile() per file
FS-->>LogsAPI: Raw entries
LogsAPI-->>Browser: JSON {entries, count}
Browser->>SessionsAPI: GET /sessions
SessionsAPI->>FS: findSessionFiles()
FS-->>SessionsAPI: Top 20 files
SessionsAPI->>FS: readFileSync() first line per file
FS-->>SessionsAPI: Session metadata
SessionsAPI-->>Browser: JSON {sessions}
Last reviewed commit: 89a159a |
| </div> | ||
|
|
||
| <div class="refresh-note">Auto-refreshes every 30 seconds — <a href="/dashboard">refresh now</a> — <a href="/status">JSON</a></div> | ||
| <div class="refresh-note">Auto-refreshes every 30s — <a href="/dashboard?tab=${tab}">refresh now</a> — JSON: <a href="/status">/status</a> <a href="/logs">/logs</a> <a href="/sessions">/sessions</a></div> |
There was a problem hiding this comment.
Reflected XSS via unescaped tab parameter
The tab query parameter is interpolated directly into the HTML href attribute without escaping. An attacker could craft a URL like /dashboard?tab="><script>alert(1)</script> to inject arbitrary JavaScript. Even though this server runs behind auth and on localhost, this is a reflected XSS that could be exploited by tricking an admin into clicking a malicious link.
The fix is to escape tab with the existing esc() helper, or better yet, whitelist the allowed values so only "status" or "logs" are accepted:
| <div class="refresh-note">Auto-refreshes every 30s — <a href="/dashboard?tab=${tab}">refresh now</a> — JSON: <a href="/status">/status</a> <a href="/logs">/logs</a> <a href="/sessions">/sessions</a></div> | |
| <div class="refresh-note">Auto-refreshes every 30s — <a href="/dashboard?tab=${esc(tab)}">refresh now</a> — JSON: <a href="/status">/status</a> <a href="/logs">/logs</a> <a href="/sessions">/sessions</a></div> |
Context Used: Context from dashboard - security.mdc Cursor rule (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: control-plane/server.mjs
Line: 648
Comment:
**Reflected XSS via unescaped `tab` parameter**
The `tab` query parameter is interpolated directly into the HTML `href` attribute without escaping. An attacker could craft a URL like `/dashboard?tab="><script>alert(1)</script>` to inject arbitrary JavaScript. Even though this server runs behind auth and on localhost, this is a reflected XSS that could be exploited by tricking an admin into clicking a malicious link.
The fix is to escape `tab` with the existing `esc()` helper, or better yet, whitelist the allowed values so only `"status"` or `"logs"` are accepted:
```suggestion
<div class="refresh-note">Auto-refreshes every 30s — <a href="/dashboard?tab=${esc(tab)}">refresh now</a> — JSON: <a href="/status">/status</a> <a href="/logs">/logs</a> <a href="/sessions">/sessions</a></div>
```
**Context Used:** Context from `dashboard` - security.mdc Cursor rule ([source](https://app.greptile.com/review/custom-context?memory=7155b921-a804-48fb-b2b8-e978e1c0cfa1))
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed — whitelisted tab param to ["status", "logs"] so invalid values fall back to "status". No raw input reaches the HTML.
|
|
||
| app.get("/logs", (req, res) => { | ||
| const session = req.query.session || undefined; | ||
| const lines = Math.min(parseInt(req.query.lines || "50", 10), 500); |
There was a problem hiding this comment.
NaN propagation from invalid lines query
If req.query.lines is a non-numeric string (e.g. ?lines=abc), parseInt returns NaN. Math.min(NaN, 500) is NaN, which propagates into getRecentLogs where allEntries.slice(0, NaN) returns an empty array. This won't crash, but silently returns 0 entries instead of the default 50 — which is confusing for callers.
| const lines = Math.min(parseInt(req.query.lines || "50", 10), 500); | |
| const lines = Math.min(parseInt(req.query.lines, 10) || 50, 500); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: control-plane/server.mjs
Line: 427
Comment:
**`NaN` propagation from invalid `lines` query**
If `req.query.lines` is a non-numeric string (e.g. `?lines=abc`), `parseInt` returns `NaN`. `Math.min(NaN, 500)` is `NaN`, which propagates into `getRecentLogs` where `allEntries.slice(0, NaN)` returns an empty array. This won't crash, but silently returns 0 entries instead of the default 50 — which is confusing for callers.
```suggestion
const lines = Math.min(parseInt(req.query.lines, 10) || 50, 500);
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed — using parseInt(..., 10) || 50 so NaN falls back to the default.
| // Parse session name from first line | ||
| let sessionName = null; | ||
| try { | ||
| const firstLine = readFileSync(f.path, "utf8").split("\n")[0]; |
There was a problem hiding this comment.
Reads entire file to extract first line
readFileSync(f.path, "utf8").split("\n")[0] loads the entire JSONL session file into memory just to read the first line. Session log files can grow large over time. Consider reading only the first chunk of the file, e.g. using openSync/readSync with a small buffer, or using createReadStream with a line reader to avoid loading multi-MB files into memory 20 times per /sessions request.
Prompt To Fix With AI
This is a comment left during a code review.
Path: control-plane/server.mjs
Line: 352
Comment:
**Reads entire file to extract first line**
`readFileSync(f.path, "utf8").split("\n")[0]` loads the entire JSONL session file into memory just to read the first line. Session log files can grow large over time. Consider reading only the first chunk of the file, e.g. using `openSync`/`readSync` with a small buffer, or using `createReadStream` with a line reader to avoid loading multi-MB files into memory 20 times per `/sessions` request.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed — now reads only the first 4KB via openSync/readSync buffer instead of loading the entire file. Session first lines are well under 4KB.
Additional Comments (1)
The new Prompt To Fix With AIThis is a comment left during a code review.
Path: control-plane/server.mjs
Line: 9-14
Comment:
**Missing `/sessions` endpoint from docblock**
The new `GET /sessions` endpoint (line 432) is not listed in the file header's endpoint documentation. This docblock documents all routes for quick reference.
```suggestion
* Endpoints:
* GET /health — liveness check (no auth required)
* GET /status — agent processes, sessions, system info
* GET /config — agent configuration (secrets redacted)
* GET /logs — recent session log entries (JSON)
* GET /sessions — list recent sessions with metadata (JSON)
* GET /dashboard — server-rendered HTML overview (status + logs tabs)
```
How can I resolve this? If you propose a fix, please make it concise. |
…elist - Read only first 4KB of session files instead of loading entire file - Use || 50 fallback for non-numeric lines param (prevents NaN propagation) - Whitelist tab param to 'status'|'logs' instead of interpolating raw input
Adds session log viewing to the control plane — see what the agent is doing without SSH/tmux.
New endpoints
GET /logs?session=&lines=50— recent session log entries (JSON)GET /sessions— list recent sessions with metadataDashboard changes
Reads pi session JSONL files from the agent's home directory. Filters to useful entry types (messages, tool results, compactions) and truncates output for readability.