feat(web): add authenticated browser-hosted Tabularis UI - #676
Conversation
# Conflicts: # packages/web-ui/src/utils/sqlFoldPreview.ts # packages/web-ui/src/utils/sqlFolding.ts # packages/web-ui/tests/utils/sqlFoldPreview.test.ts # packages/web-ui/tests/utils/sqlFolding.test.ts
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| Some("png") => "image/png", | ||
| Some("jpg" | "jpeg") => "image/jpeg", | ||
| Some("webp") => "image/webp", | ||
| Some("svg") => "image/svg+xml", |
There was a problem hiding this comment.
CRITICAL: Stored XSS via user-uploaded SVG connection icons
connection_icon_asset serves .svg uploads as image/svg+xml inline with only Cache-Control and Content-Type headers — no X-Content-Type-Options: nosniff, no Content-Security-Policy, and no Content-Disposition: attachment (contrast plugin_asset, which applies PLUGIN_ASSET_CSP + nosniff + CORP). Any authenticated user can upload an SVG containing <script> via /api/v1/uploads/connection-icons; navigating an admin's browser to /api/v1/assets/connection-icons/<file>.svg executes that script in the Web UI origin, yielding stored XSS against the database admin UI. Add nosniff and a strict CSP (or force Content-Disposition: attachment and serve from a sandboxed origin) for this handler.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| ) -> Result<IssuedSession, AuthenticationError> { | ||
| let now = Instant::now(); | ||
| let mut state = self.lock_state(); | ||
| if state |
There was a problem hiding this comment.
WARNING: Global login rate-limit enables trivial global lockout DoS (proxy mode)
SecurityState.failed_logins / blocked_until are process-global, not per-identifier, and the lockout check runs before credential validation. In trusted-proxy mode every cookieless request reaches authenticate_proxy → authenticate_remote; an attacker who can merely reach the HTTP port can send 5 wrong x-tabularis-proxy-secret values within the window and set blocked_until, locking out every legitimate proxy user (including correct-secret requests) for the full lockout duration. Apply rate limiting per-identifier, and avoid applying password-style brute-force lockout to the trusted-proxy flow.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| ); | ||
| return status_response(StatusCode::UNAUTHORIZED); | ||
| }; | ||
| match state.security.authenticate_proxy(secret, user) { |
There was a problem hiding this comment.
WARNING: Proxy auth mints a fresh session on every cookieless request
When no valid session cookie is present, authenticate_proxy → issue_session inserts a new SessionRecord on each request, and the response always sets a new session cookie. The session store has no capacity cap (unlike WebEventBus::max_sessions); sessions are only pruned at TTL expiry. A reverse proxy that does not round-trip the cookie — or any holder of the proxy secret — can exhaust server memory by minting unbounded sessions over their TTL. Reuse an existing session for an already-authenticated proxy identity, or bound the session store.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| Ok(Err(InvocationError::Application(error))) => failure( | ||
| metadata.application_error_status, | ||
| metadata.application_error_code, | ||
| error.message, |
There was a problem hiding this comment.
WARNING: Application error messages/details forwarded verbatim to clients
InvocationError::Application(error) propagates error.message and error.details directly into the stable RPC error response. Downstream producers build these strings from fs/IO errors (map_err(|e| e.to_string())) and driver errors, which include absolute server filesystem paths, SQL text, and potentially credential-bearing connection strings. This leaks server internals to remote clients. Sanitize/replace the message at the RPC boundary (keep the stable application_error_code, drop or genericize the free-text message/details).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| pub const RPC_CANCELLATION_HEADER_NAME: &str = "x-tabularis-cancellation-id"; | ||
|
|
||
| const DEFAULT_DEADLINE: Duration = Duration::from_secs(30); | ||
| const MAX_DEADLINE: Duration = Duration::from_secs(6 * 60 * 60); |
There was a problem hiding this comment.
WARNING: Client-controlled 6 h deadline enables resource-hold DoS
MAX_DEADLINE is 6 hours and the deadline is taken from the client-supplied x-tabularis-deadline-ms header (capped only at this value). An authenticated remote session can set a near-6 h deadline and issue long-running queries to occupy its full per-session concurrency budget (16) and a chunk of the global budget (64) for hours. Six hours is far beyond a reasonable web-RPC bound; lower the cap and consider a separate, smaller bound for remote sessions.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| headers.insert( | ||
| CONTENT_DISPOSITION, | ||
| HeaderValue::from_str(&content_disposition) | ||
| .expect("sanitized file names are valid header values"), |
There was a problem hiding this comment.
WARNING: Content-Disposition header panics on unsanitized filename
metadata.file_name ultimately derives from the client-supplied x-tabularis-file-name upload header (url-decoded, no sanitization) and is interpolated into Content-Disposition then parsed with HeaderValue::from_str(...).expect(...). A filename containing ", \r, or \n makes from_str return Err, panicking the handler task (per-request DoS) instead of returning an error response. Validate/sanitize the filename (strip control bytes and quotes, or use from_str with a graceful fallback).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| events: WebEventBus, | ||
| ) -> Router { | ||
| let index = web_root.join("index.html"); | ||
| let static_files = ServeDir::new(web_root).fallback(ServeFile::new(index)); |
There was a problem hiding this comment.
WARNING: No CSP/hardening headers on the SPA shell
The app shell is served via ServeDir::new(web_root).fallback(ServeFile::new(index)) with no Content-Security-Policy, X-Content-Type-Options, Referrer-Policy, or Cross-Origin-Resource-Policy. The login page (login_page) and plugin assets set CSP, but the actual admin SPA — which handles credentials and DB data — does not, leaving no defense-in-depth mitigation if an XSS reaches the bundle. Add a baseline CSP + nosniff for the static fallback.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| return Ok(Vec::new()); | ||
| }; | ||
| let selected: HashSet<&str> = events.iter().map(String::as_str).collect(); | ||
| Ok(session |
There was a problem hiding this comment.
SUGGESTION: Reconnect silently drops events evicted from bounded history
When a client subscribes with since, only events still present in the bounded history deque (capacity 128) are returned; events between since and the oldest retained entry are silently lost, and the subscribe result carries no gap/watermark indicator, so the client cannot detect data loss and resync. Return a high-watermark or missed flag so clients can refetch full state after a gap.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 8 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (this pass)Incremental pass for Files reviewed this pass:
Fix these issues in Kilo Cloud Previous Review Summaries (7 snapshots, latest commit 64472f0)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 64472f0)Status: 8 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (5 files)
Incremental pass for commit Fix these issues in Kilo Cloud Previous review (commit fc0f3ba)Status: 8 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Incremental Review (commit fc0f3ba)This update swaps the SPA shell favicon from Files reviewed this pass:
Fix these issues in Kilo Cloud Previous review (commit 1c32773)Status: 8 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Incremental Review (commit 1c32773)This update adds a server-side file browser for the Web UI: a new The new code is security-conscious: roots and entries are canonicalized, path containment is enforced via Files reviewed this pass:
Fix these issues in Kilo Cloud Previous review (commit 55bf5db)Status: 8 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Incremental Review (commit 55bf5db)This update refactors the Web launch surface from a Files reviewed this pass:
Fix these issues in Kilo Cloud Previous review (commit 9e75c18)Status: 8 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Incremental Review (commit 9e75c18)This update added a connection-scoped editor route ( Files reviewed this pass:
Fix these issues in Kilo Cloud Previous review (commit 3d9d0ef)Status: 8 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Incremental Review (commit 3d9d0ef)This update added SQL code-folding + hover-preview in the web UI and a cross-platform Files reviewed this pass:
Fix these issues in Kilo Cloud Previous review (commit f308c3f)Status: 8 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Scope & Files ReviewedThis PR is a large architectural change (~1,040 files, frontend package move + new Web transport). Review focused on the highest-risk new security-critical code paths rather than relocated/renamed files:
Verified-safe areas (no comment): bootstrap token single-use + 60 s TTL + 244-bit entropy; cookie flags ( Reviewed by glm-5.2 · Input: 75.8K · Output: 21.2K · Cached: 1.5M |
1 participant
Summary
This PR introduces Tabularis Web, a browser-hosted mode that reuses the existing React UI and Rust application services while preserving the native Tauri desktop application.
Users can start a local authenticated browser session with:
The implementation adds a shared typed application boundary, an authenticated HTTP/WebSocket transport, browser-specific platform adapters, and broad desktop-to-browser feature parity. Running Tabularis without the
websubcommand keeps the existing desktop behavior.Why
Tabularis was previously coupled to the Tauri IPC and native-window runtime. This prevented the application from being operated through a browser, deployed as a headless service, or tested against a transport-independent application contract.
This change separates application services from transport concerns so that:
Architecture
Shared frontend package
packages/web-uias@tabularis/web-ui.Typed application client
TabularisClientProviderused by application features.Shared Rust application layer
src-tauri/src/applicationservices.Web transport
Feature coverage
The browser mode covers the main Tabularis workflows:
Browser-only adaptations are capability-driven and provide explicit fallbacks for file dialogs, downloads, clipboard permissions, notifications, external links, secondary windows, plugin install links, and updater behavior.
Security model
Local mode
127.0.0.1by default.HttpOnly,SameSite=Strictsession.Host,Origin, and CSRF state.Remote mode
Non-loopback deployment is rejected unless remote authentication is fully configured. Supported modes are:
TABULARIS_WEB_PASSWORD;TABULARIS_WEB_PROXY_SECRET.Remote deployments require an HTTPS public URL and explicit allowed origins. Sessions receive database-only permissions by default. Host-level operations remain denied unless the operator explicitly enables
--allow-high-risk; MCP remains unavailable remotely.Additional protections include login rate limiting, secure cookies, strict origin handling, bounded request/result sizes, session ownership checks, scoped transfer tokens, cleanup on expiry/logout/shutdown, and restricted plugin asset delivery with CSP headers.
CLI and deployment
New Web options include:
web(subcommand)--host--port--no-open--web-root--auth--public-url--allowed-origin--allow-high-riskPackaged browser assets are verified for release artifacts. The operator documentation covers local use, reverse proxies, systemd, containers, credential storage, upgrades, rollback, troubleshooting, plugin trust, and browser limitations.
Testing and verification
The branch completion ledger records the following final gates:
pnpm lintpnpm typecheckpnpm test:coverage— 270 files / 4,023 testspnpm test:rust— 1,292 passed / 4 ignoredpnpm build@tabularis/web-uiThe latest CI run for
feat/web-uicompleted successfully before this PR was opened.Documentation
web-ui-project/docs/WEB_MODE_OPERATIONS.mdweb-ui-project/docs/WEB_REMOTE_SECURITY.mdweb-ui-project/docs/WEB_PERFORMANCE_RESILIENCE.mdweb-ui-project/docs/WEB_MANUAL_PARITY_AUDIT.mdweb-ui-project/docs/WEB_MODE_UPGRADES.mdweb-ui-project/docs/architecture/decisions/web-ui-project/tasks/PROGRESS.mdReview guide
This is intentionally a large architectural change. The recommended review order is:
Areas with the highest regression risk are shared runtime bootstrap, connection/query lifecycle, persistence, tunnels, plugin management, file transfers, and desktop/Web transport parity.
Compatibility and operational notes
Scope
This PR contains 49 commits and changes approximately 1,040 files, including the frontend package move. Reviewers should use rename-aware diffs where possible to distinguish relocated files from behavioral changes.