Add filter-mode parity (Last N Lines / Time Range / All) to Chat Logs modal

[dngrtech]

Summary

Brings the Chat Logs modal to feature parity with the Instance Logs modal by adding the Filter By control: Last N Lines / Time Range / All.

• Shared UI: extracts the filter bar into LogFilterControls.jsx + logFilterOptions.js, now used by both modals (removes duplication; both modal files stay under the 300-line limit).
• Behavior parity: filter changes (mode / lines / time) apply via the Apply button instead of refetching on every click. Selecting a different archive file (chat.log, chat.log.N) still reloads immediately. Header subtitle shows the active filter.
• Full-stack filter_mode for chat logs:
  • lines → tail -n
  • all → cat
  • time → host-computed date -d "<since>" cutoff, compared with awk against each line's fixed-width [YYYY-MM-DD HH:MM:SS] prefix. Scoped to the selected archive file.
• Docs: docs/api_reference.md updated for the new chat-logs query params.
• Version: bumped to 1.13.12 (VERSION, version.json).

Security

The time-range task runs a shell command as root on the QLDS host. Hardened against command injection (flagged by automated review):

• Server-side allowlist for since (fixed set of windows) and a strict chat.log(.N) regex for filename.
• SINCE / LOGFILE passed to the shell via environment: env vars, so Jinja values are never expanded into the command text. Verified locally: a malicious since yields only date: invalid date and executes nothing.

Test plan

• pnpm/vitest: all 53 frontend tests pass; ESLint clean; production build succeeds.
• awk/date time filter unit-tested locally against a synthetic chat.log (entries before cutoff dropped, at/after kept, connection-event lines included).
• Injection neutralization verified locally (no PWNED file created).
• ansible-playbook --syntax-check passes; Python compiles.
• Live click-through against a real instance (needs the production host to fetch remote chat logs).

[github-actions]

PR Review: Filter-mode parity for Chat Logs modal

Strengths

• Clean DRY extraction: LogFilterControls.jsx and logFilterOptions.js correctly centralize all filter UI and constants, eliminating the ~80-line duplicated block that previously lived in ViewLogsModal.jsx. Both modals now share a single source of truth.
• Solid injection prevention in Ansible: SINCE and LOGFILE are passed as environment variables (not shell-interpolated), so the awk/date command cannot be exploited even if upstream validation were bypassed (ansible/playbooks/fetch_chat_logs.yml:56-58).
• Strong input validation in the route: The filter_mode enum check, ALLOWED_CHAT_LOG_SINCE frozenset allowlist, and the filename regex (re.fullmatch(r'chat\.log(\.\d+)?', filename)) together close the obvious injection surfaces (ui/routes/instance_routes.py:616-629).
• Apply-on-demand UX is consistent: The useEffect correctly removes filterMode/lineCount/timeRange from its dependency array so filter changes don't trigger live reloads — intentional and well-commented.
• getFilterDescription lifted to shared module: Eliminates the local inline version that was in ViewLogsModal.jsx and keeps the header text consistent across both modals.

---

Issues

Critical (Must Fix)

None.

---

Important (Should Fix)

1. No tests for the new validation logic
ui/routes/instance_routes.py:616–629 adds four distinct validation branches (filter_mode enum, filename regex, since allowlist, lines range bypass for 'all'). None of this is covered by automated tests. If the allowlist or regex is accidentally mutated, there is nothing to catch it.

• Add route-level unit tests covering: valid modes, invalid filter_mode, invalid filename (e.g. ../etc/passwd, chat.log.abc), invalid since for time mode, valid since for non-time mode (should pass without error), and boundary values for lines.

2. Lines are validated for time mode but never used
ui/routes/instance_routes.py:632:

if filter_mode != 'all' and (lines < 10 or lines > 10000):

This rejects an out-of-range lines value even when filter_mode == 'time', where lines is ignored by the playbook. An innocent caller setting filter_mode=time&lines=0 (or not passing lines at all, which defaults to 500 and passes) would be confused by a 400 error unrelated to the actual operation. Restrict the check to filter_mode == 'lines' only, or document that lines is ignored for other modes.

3. leading prop is dead code
frontend-react/src/components/instances/LogFilterControls.jsx:125:

leading = null,

The prop is accepted and rendered but never passed by either consumer. The chat log modal's file selector Listbox is rendered in the modal header, not via leading. Either wire it up or remove it; dead API surface in a shared component creates confusion about what callers should do.

---

Minor (Nice to Have)

4. since is always sent in API params regardless of mode
frontend-react/src/services/api.js:558-559:

const params = new URLSearchParams({
  filter_mode: filterMode,
  since: since,
  ...

When filterMode is 'lines' or 'all', the since value is sent to the backend, which ignores it. This is harmless (backend validates it only for time mode) but adds noise to the request URL and makes logs harder to read. Only include since when filterMode === 'time'.

5. No server-side cap on response size for all mode
ansible/playbooks/fetch_chat_logs.yml:38-44 — cat {{ log_file }} has no size guard. A multi-GB log file would be streamed entirely through Ansible stdout, into Python memory, and returned over HTTP. The UI warning is good, but the server has no protection. The pre-existing instance-logs endpoint has the same issue, so this is not a regression — but extending the pattern to chat logs makes it worth flagging. A head -c 50M or similar hard cap would bound the blast radius.

6. filter_mode not validated at the Ansible layer
ansible/playbooks/fetch_chat_logs.yml — if log_mode receives a value other than 'lines', 'all', or 'time' (only possible by bypassing the Python route), all three tasks are skipped and the output variable falls through to the chat_output_lines.stdout | default('') branch, returning empty string with no error. This is a graceful silent failure rather than a security issue, but a failing assertion or assert task at the start of the play would make the misconfiguration visible.

---

Assessment

Ready to merge? Yes, with fixes recommended.

Reasoning: The security-sensitive path (injection prevention via env vars + input allowlists + filename regex) is handled correctly, and the refactoring is clean. The two "Important" issues (missing tests, misleading lines-validation-for-time-mode) are worth addressing before or shortly after merge, but neither is a data-loss risk or vulnerability.