← Back to PRs

#16061: fix(sessions): tolerate invalid sessionFile metadata

by haoyifan open 2026-02-14 07:20 View on GitHub →
stale size: XS
fix(sessions): tolerate invalid sessionFile metadata AI-assisted: yes (Codex/OpenClaw agent). Problem - Some deployments end up with `sessions.json` entries containing a `sessionFile` field that is stale/corrupted (e.g., absolute paths persisted historically, state dir migrations, or other bad metadata). - `resolveSessionFilePath()` currently treats an invalid `sessionFile` as fatal and throws: "Session file path must be within sessions directory". - When this happens inside chat/channel handlers (observed with Telegram), the handler can fail mid-update, leading to silent non-replies even though the message was received. Fix - Treat `entry.sessionFile` as optional, best-effort metadata. - If validation fails, ignore it and fall back to the derived safe transcript path based on `sessionId` (which is already validated and resolved within the agent sessions directory). Security - Containment checks are still enforced when a candidate is provided. - This change does NOT allow escaping paths; it simply avoids taking down message handling due to invalid metadata. Testing - Local (Istanbul VM): - `pnpm format:check` PASS - `pnpm check` PASS - `pnpm vitest run src/config/sessions/paths.test.ts` PASS Concise output: - "✓ src/config/sessions/paths.test.ts (12 tests)" - Real-world repro: - Reproduced intermittent Telegram handler failures caused by invalid/stale `sessionFile` metadata, and confirmed this change prevents silent non-replies by falling back to the derived safe transcript path. <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR makes `resolveSessionFilePath` resilient to corrupted or stale `sessionFile` metadata in `sessions.json`. Instead of throwing (which could crash message handlers mid-update, causing silent non-replies on Telegram and other channels), invalid candidates now fall back to the derived safe transcript path based on `sessionId`. - `resolveSessionFilePath` in `paths.ts` now wraps `resolvePathWithinSessionsDir` in a try/catch; if the persisted `sessionFile` fails validation (path traversal, absolute path outside sessions dir, etc.), it silently falls back to `{sessionId}.jsonl` - Security containment checks are still enforced — the change does not allow path escapes, it just avoids a fatal error on bad metadata - Tests updated from expecting throws to verifying fallback behavior, while still validating that legitimate candidates within the sessions dir are accepted - Multiple callers (e.g., `get-reply-run.ts`, `agent-runner.ts`, `doctor-state-integrity.ts`) don't have their own try/catch around this function, confirming the fix is necessary at the source <h3>Confidence Score: 5/5</h3> - This PR is safe to merge — it strictly improves resilience without weakening any security checks. - The change is minimal and well-scoped: a single try/catch in `resolveSessionFilePath` with a safe fallback. Security containment checks remain enforced for valid candidates. The fallback path is derived from `sessionId` which is validated by `validateSessionId` (alphanumeric + `._-` only). Tests cover all key scenarios. No new dependencies or architectural changes. - No files require special attention. <sub>Last reviewed commit: c06f155</sub> <!-- greptile_other_comments_section --> <sub>(2/5) Greptile learns from your feedback when you react with thumbs up/down!</sub> <!-- /greptile_comment -->

Most Similar PRs