Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions TECH_DEBT.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,15 @@ Completed query snapshots (including result rows) are cleaned up after `STACKABL

### Client-side row accumulation has no memory bound

**File:** `src/routes/(app)/trino/query-runner.svelte.ts`
**File:** `src/lib/trino/query-runner.svelte.ts`

The client accumulates all result rows in memory up to `MAX_CLIENT_ROWS` (10,000). For wide result sets this could consume significant browser memory. Consider implementing streaming/virtual scrolling for large results.

---

### Displayed results not cleared on connection change

**File:** `src/routes/(app)/trino/+page.svelte`
**File:** `src/routes/(app)/trino/query-runner.svelte.ts`, `src/routes/(app)/trino/+page.svelte`

After saving a new connection, the previous query results remain visible until a new query is run. Consider calling `runner.reset()` when the connection changes.

Expand All @@ -122,6 +122,34 @@ Mobile viewport tests (393×851, touch-enabled) are excluded from CI runs to red

## Infrastructure

### Session cookie not configured for cross-origin iframe embedding

**File:** `src/lib/server/auth.ts`

The better-auth session cookie currently uses `SameSite=Lax` (the browser default when no `SameSite` attribute is set). Browsers do not send `SameSite=Lax` cookies when a page is loaded inside an `<iframe>` whose top-level frame is on a different origin, so authenticated users visiting `/trino?embed=1` or `/storage?embed=1` from an external host page will be silently redirected to `/auth/login`.

To fix this, configure the session cookie with `SameSite=None; Secure` in `better-auth`:

```typescript
// src/lib/server/auth.ts
export const auth = betterAuth({
advanced: {
cookies: {
session_token: {
attributes: { sameSite: 'none', secure: true }
}
}
}
// …rest of config
});
```

**Why this is deferred:** `SameSite=None` is only accepted by browsers when the `Secure` flag is also set, which requires HTTPS. The current dev setup uses plain HTTP, so setting this unconditionally would break local development. The correct approach is to make it conditional on a production/HTTPS flag (e.g. `process.env.NODE_ENV === 'production'` or a dedicated env var) once a staging environment with HTTPS is available.

**Also required for cross-origin embedding:** The embedding host page and the Cockpit server must both be on HTTPS, and the Cockpit server's CORS / CSP must explicitly trust the embedding origin if additional API restrictions are in place.

---

### No Content Security Policy headers

No CSP headers are set anywhere. This leaves the app exposed to XSS in ways that a strict CSP would mitigate. Should be added in a SvelteKit hook once the app stabilises.
Expand Down
43 changes: 36 additions & 7 deletions src/hooks.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,26 +69,55 @@ const handleAuthGuard: Handle = async ({ event, resolve }) => {
* malformed / invalid header on any route.
*/
const handleStorageConnection: Handle = async ({ event, resolve }) => {
if (
(event.route.id?.startsWith('/(app)/storage/') ||
event.route.id?.startsWith('/(app)/api/storage/')) &&
!storageBrowserEnabled
) {
const routeId = event.route.id ?? '';
const isAppStorage = routeId.startsWith('/(app)/storage/');
const isStorageApi = routeId.startsWith('/(app)/api/storage/');

if ((isAppStorage || isStorageApi) && !storageBrowserEnabled) {
throw error(404, 'Storage browser is not enabled');
}
event.locals.storageConfig = getConnectionFromHeader(event.request);
if (event.locals.storageConfig === null && event.route.id?.startsWith('/(app)/api/storage/')) {
if (event.locals.storageConfig === null && isStorageApi) {
throw error(401, 'No storage connection configured');
}
return resolve(event);
};

/**
* Allow pages loaded with `?embed=1` to be displayed inside <iframe> elements
* from any origin. The `(app)` layout uses the same query parameter to hide
* the app shell (sidebar/header) so a single module can be embedded on its
* own.
*
* By default browsers block cross-origin framing when the server sets
* `X-Frame-Options: SAMEORIGIN` or a restrictive `frame-ancestors` CSP.
* SvelteKit does not set either header by default, so this hook is mainly a
* defence-in-depth measure and an explicit signal that embedding is intended.
*
* For cross-origin embedding to work the session cookie must also carry
* `SameSite=None; Secure`. See TECH_DEBT.md for the outstanding action item.
*/
const handleEmbedHeaders: Handle = async ({ event, resolve }) => {
const response = await resolve(event);
if (event.url.searchParams.get('embed') === '1') {
response.headers.set('X-Frame-Options', 'ALLOWALL');
response.headers.set(
'Content-Security-Policy',
[response.headers.get('Content-Security-Policy'), 'frame-ancestors *']
.filter(Boolean)
.join('; ')
);
}
return response;
};

export const handle = sequence(
requestLogger,
handleMetrics,
handleParaglide,
...(oidcEnabled ? [handleAuth, handleAuthGuard] : []),
handleStorageConnection
handleStorageConnection,
handleEmbedHeaders
);

export const handleError: HandleServerError = ({ error, event, status, message }) => {
Expand Down
79 changes: 63 additions & 16 deletions src/routes/(app)/+layout.svelte
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<script lang="ts">
import { browser } from '$app/environment';
import { beforeNavigate } from '$app/navigation';
import { page } from '$app/state';
import * as m from '$lib/paraglide/messages.js';
import Sidebar from '$lib/components/layout/sidebar/Sidebar.svelte';
Expand Down Expand Up @@ -36,31 +37,77 @@
(page.url.pathname.startsWith('/storage') ? m.page_title_storage : m.page_title_default)
)()
);

// ── Embed mode ───────────────────────────────────────────────────────────
// Loading any page with `?embed=1` hides the app shell (sidebar + header)
// so the module can be embedded in an iframe of another frontend. The mode
// is sticky per tab via sessionStorage so it survives full-page navigations
// (e.g. form submissions), and the query parameter is kept in the URL for
// client-side navigations via `beforeNavigate`.
const EMBED_KEY = 'cockpit_embed';
const embedParam = $derived(page.url.searchParams.get('embed') === '1');
let embedSticky = $state(false);

$effect(() => {
if (!browser) return;
if (embedParam) {
try {
sessionStorage.setItem(EMBED_KEY, '1');
} catch {
// sessionStorage unavailable (e.g. blocked); URL param still works
}
embedSticky = true;
} else {
try {
embedSticky = sessionStorage.getItem(EMBED_KEY) === '1';
} catch {
embedSticky = false;
}
}
});

const isEmbed = $derived(embedParam || embedSticky);

// Keep `?embed=1` in the URL for client-side navigations so reloads and
// open-in-new-tab keep the embedded layout.
beforeNavigate((nav) => {
if (isEmbed && nav.to?.url) {
nav.to.url.searchParams.set('embed', '1');
}
});
</script>

<svelte:head>
<title>{title} | {m.page_title_suffix()}</title>
</svelte:head>

<div class="bg-base-100 flex h-dvh overflow-hidden">
<Sidebar
bind:collapsed={sidebarCollapsed}
bind:mobileOpen
storageBrowserEnabled={data.storageBrowserEnabled}
/>

<div class="flex min-w-0 flex-1 flex-col">
<Header
{title}
{mobileOpen}
user={data.user}
onToggleMobile={() => (mobileOpen = !mobileOpen)}
/>

{#if isEmbed}
<div class="bg-base-100 flex h-dvh overflow-hidden">
<main class="bg-base-200 flex-1 overflow-auto p-6">
{@render children()}
</main>
</div>
</div>
{:else}
<div class="bg-base-100 flex h-dvh overflow-hidden">
<Sidebar
bind:collapsed={sidebarCollapsed}
bind:mobileOpen
storageBrowserEnabled={data.storageBrowserEnabled}
/>

<div class="flex min-w-0 flex-1 flex-col">
<Header
{title}
{mobileOpen}
user={data.user}
onToggleMobile={() => (mobileOpen = !mobileOpen)}
/>

<main class="bg-base-200 flex-1 overflow-auto p-6">
{@render children()}
</main>
</div>
</div>
{/if}

<ToastHost />
Loading