From 681ac1fa6bed03a8c7c54f0bdbc1c56c77a0cb8e Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Thu, 6 Aug 2026 00:16:03 -0400 Subject: [PATCH 1/9] fix(auth): return to the intended page after login instead of the dashboard PrivateRoute redirected to /login with no record of where the visitor was going, and every post-auth path then navigated to '/'. Any deep link opened without a session was silently discarded -- you landed on the dashboard with no explanation of why the thing you clicked did not happen. That was survivable when deep links were rare. It is not now: serverkit.ai install links (/extensions?install=, /templates?install=) arrive from README badges, and by construction they are clicked by people who may have no open session. Every first-time click hit this path. The destination goes in sessionStorage rather than react-router location state because the SSO flow leaves the origin entirely for the identity provider and returns through /login/callback/; router state cannot survive that. One mechanism covering all four post-auth exits (login link redemption, password, 2FA, SSO) beats two each covering half. sanitizeRedirect is the pure half and is validated on write AND on read. Login is exactly where an open redirect hurts, so `//evil.com` and `/\evil.com` -- both of which browsers read as protocol-relative -- are rejected along with control characters and the auth routes themselves, which would loop. Proving test: node --test src/utils/__tests__/redirectAfterLogin.test.mjs (9 tests). Co-Authored-By: Claude Opus 5 (1M context) --- frontend/src/App.jsx | 11 ++- frontend/src/pages/Login.jsx | 7 +- frontend/src/pages/SSOCallback.jsx | 5 +- .../__tests__/redirectAfterLogin.test.mjs | Bin 0 -> 3580 bytes frontend/src/utils/redirectAfterLogin.js | 72 ++++++++++++++++++ 5 files changed, 90 insertions(+), 5 deletions(-) create mode 100644 frontend/src/utils/__tests__/redirectAfterLogin.test.mjs create mode 100644 frontend/src/utils/redirectAfterLogin.js diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index af4f6469..66048e24 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -6,6 +6,7 @@ import { ThemeProvider } from './contexts/ThemeContext'; import { LayoutProvider } from './contexts/LayoutContext'; import { ResourceTierProvider } from './contexts/ResourceTierContext'; import { NotificationsProvider } from './contexts/NotificationsContext'; +import { rememberRedirect } from './utils/redirectAfterLogin'; import { Toaster } from './components/ui/sonner'; import ThemeSync from './components/ThemeSync'; import DashboardLayout from './layouts/DashboardLayout'; @@ -214,6 +215,7 @@ function DevOnlyRoute({ children }) { function PrivateRoute({ children }) { const { isAuthenticated, loading, needsSetup, needsMigration } = useAuth(); + const location = useLocation(); if (loading) { return ; @@ -228,7 +230,14 @@ function PrivateRoute({ children }) { return ; } - return isAuthenticated ? children : ; + if (isAuthenticated) return children; + + // Park the destination so login can return to it. Deep links into the + // panel (?install= from a serverkit.ai badge, a shared /servers/:id) are + // routinely opened without a session, and landing on the dashboard with + // no explanation is the worst version of that. + rememberRedirect(location); + return ; } function PublicRoute({ children }) { diff --git a/frontend/src/pages/Login.jsx b/frontend/src/pages/Login.jsx index 7d4b8034..76e81b66 100644 --- a/frontend/src/pages/Login.jsx +++ b/frontend/src/pages/Login.jsx @@ -5,6 +5,7 @@ import api from '../services/api'; import SSOProviderIcon from '../components/SSOProviderIcon'; import ServerKitLogo from '../components/ServerKitLogo'; import AuthLayout from './auth/AuthLayout'; +import { consumeRedirect } from '../utils/redirectAfterLogin'; import { Button } from '@/components/ui/button'; import { Input } from '@/components/ui/input'; import { Label } from '@/components/ui/label'; @@ -54,7 +55,7 @@ const Login = () => { api.redeemLoginLink(token) .then((response) => { setUser(response.user); - navigate('/', { replace: true }); + navigate(consumeRedirect(), { replace: true }); }) .catch((err) => { setError(err.message || 'Invalid or expired login link'); @@ -110,7 +111,7 @@ const Login = () => { // No 2FA - complete login setUser(response.user); - navigate('/'); + navigate(consumeRedirect()); } catch (err) { setError(err.message || 'Failed to login'); } finally { @@ -138,7 +139,7 @@ const Login = () => { console.warn(response.warning); } - navigate('/'); + navigate(consumeRedirect()); } catch (err) { setError(err.message || 'Invalid verification code'); // Clear the code inputs on error diff --git a/frontend/src/pages/SSOCallback.jsx b/frontend/src/pages/SSOCallback.jsx index a1fd9c66..f2d6512d 100644 --- a/frontend/src/pages/SSOCallback.jsx +++ b/frontend/src/pages/SSOCallback.jsx @@ -2,6 +2,7 @@ import { useEffect, useState } from 'react'; import { useParams, useNavigate, useSearchParams, Link } from 'react-router-dom'; import { useAuth } from '../contexts/AuthContext'; import api from '../services/api'; +import { consumeRedirect } from '../utils/redirectAfterLogin'; import { Loader } from 'lucide-react'; import { Button } from '@/components/ui/button'; @@ -41,7 +42,9 @@ const SSOCallback = () => { } setUser(response.user); - navigate('/'); + // sessionStorage survived the round trip to the identity provider; + // react-router state would not have. + navigate(consumeRedirect()); } catch (err) { setError(err.message || 'SSO authentication failed'); } diff --git a/frontend/src/utils/__tests__/redirectAfterLogin.test.mjs b/frontend/src/utils/__tests__/redirectAfterLogin.test.mjs new file mode 100644 index 0000000000000000000000000000000000000000..8de936259853289f9421945118706e14722fbfc2 GIT binary patch literal 3580 zcmbtX+iv4F5bblmVt@d34A>UBy`XTKUG%9aun+A%8weO_Y%!)tEjhBC1_AmJ{lb1p z52?tp6W5S-jR00i&S>V$;hAxk(I4Jzv|W*K5L!A>|tXM?qOF`Y-;}oz|gs)Oe;+nI*a9-qo*5VgE6DpHQfc7IKYHaD*q{ z*bp5ywqQ@SQ3%1hg6S%#iYb(;W>T$loE04xi}i0xIK$?zkoZ7TeIpuTkiAon3!$Jbqr*331KdoHcEQ&rbgD=9}4x*F5XdwRSeeA<`K_;aHS zYJW_xt5L=~VH-fkFd9$j`2Xu(9$LH6xZi1jvtb{y@3VqMY13N2UA?||g2$s|BSk(p z>>oOU{rmSni8tHQ83r*_z?}j)1MyCyoadlOsYQU!V)8*-(A=1tHo&!(>DKw8_8cN! zdnbpuowaFAPGPn8bs`|)3Rf6~-}|;d+i!EMtek(iQH{7NU9sbs`kU>;IthBI!cbZj zE)SOhw;arBOMJZ2k1Qt>xD;2nF3)Tq(Wlj$0PS1`Eqo}!uQ>t@fs%{Motb(D=6b_( zy7O)e287750wN=2)M4;3BoS4RsS+~lyoMOhdfl3gH`>g2r%+j*LE&ObwlU@uH%?IE zjs2^eJiygr@pZA3B6XN&Zz39&QS|cFPVUu4h1}~}=E^{Zuf7`(ivrzPLC|-Sl~S3> zFRiop2Tl%dNXIK>AKI1}+m*6;nYy4tr+{L4=j5Cay{+xBB=;{Qn#Px!?sa8%KGWP; z@y<|QDz9>kAFuCE|8}!Ty3jwnHn)ZBd(}|WWcbZNe#6@1!Y}Mc2hD||%_))nz}E>@ z#NtDlOE92;bWdno>b#__YfOO|2O>7CQ>U@$;FW#C_q;+QWIxa>urx_y2Vh+3l}AGw z_$DL;bY2-jg#V0vK8uZKKi^TPn$-%89+P0gHBLJic`znMsPmHZbwBM*NPe)mEH__# zW3uGr@_+6N=CT-^ln{Z6(MaOVqPzs!YOHq|Bxc0v8b1k)p{2GwkQAv>(^aF3g#0Kw zTIknhQzp;jU-x{ne_Cg!?QD<96T>~G645+R0$$bnykI2$4d{#U=T1bi7}89CcsJzq L, +// /templates?install=) arrive from README badges and are, by definition, +// clicked by people who may not have an open session. Dropping the query +// string on the way through login made every one of those links land on a +// bare dashboard with no explanation. +// +// sessionStorage rather than react-router's location.state: the SSO flow +// leaves the origin entirely for the identity provider and comes back through +// /login/callback/, and router state cannot survive that. One +// mechanism that covers every path beats two that each cover half. + +const STORAGE_KEY = 'serverkit.redirectAfterLogin'; + +// Landing back on one of these after login is either a loop or nonsense. +const AUTH_PATHS = ['/login', '/register', '/setup', '/migrate', '/logout']; + +/** + * Validate a stored destination before we navigate to it. + * + * Returns the path, or null when it is unusable. Same-origin is enforced by + * shape: a destination must be one absolute path and nothing else. `//evil.com` + * and `/\evil.com` are the two that matter — browsers read both as + * protocol-relative URLs, so either would turn login into an open redirect. + */ +export function sanitizeRedirect(path) { + if (typeof path !== 'string' || !path) return null; + if (path[0] !== '/') return null; + if (path[1] === '/' || path[1] === '\\') return null; + // Control characters (newline, tab, NUL) can smuggle a value past the + // shape checks above once something downstream re-parses it. + // eslint-disable-next-line no-control-regex + if (/[\u0000-\u001f\u007f]/.test(path)) return null; + + const pathname = path.split(/[?#]/)[0]; + if (AUTH_PATHS.includes(pathname)) return null; + + return path; +} + +/** Store where the visitor was going. Call this before redirecting to /login. */ +export function rememberRedirect(location) { + if (!location) return; + const path = sanitizeRedirect( + `${location.pathname || ''}${location.search || ''}${location.hash || ''}`, + ); + if (!path || path === '/') return; // the dashboard is already the default + try { + window.sessionStorage.setItem(STORAGE_KEY, path); + } catch { + // Storage unavailable — fall back to the dashboard, as before. + } +} + +/** + * Read and clear the stored destination, falling back to the dashboard. + * Re-validates on the way out: the value is same-origin sessionStorage, but a + * post-login navigation is not the place to trust that assumption. + */ +export function consumeRedirect() { + let stored = null; + try { + stored = window.sessionStorage.getItem(STORAGE_KEY); + window.sessionStorage.removeItem(STORAGE_KEY); + } catch { + return '/'; + } + return sanitizeRedirect(stored) || '/'; +} From 6b4e7db24293760d623bd9554f597747a700cca6 Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Thu, 6 Aug 2026 00:16:16 -0400 Subject: [PATCH 2/9] feat(extensions): open an extension from /extensions?install= The counterpart to Templates.jsx's ?install=, which has worked for a while; the extensions page never grew one, so serverkit.ai install links and README badges had nowhere to land. It opens the extension's detail modal rather than installing. A URL arriving from another site must not be able to install anything on its own -- the operator still presses Install, and the trust gates behind it (unreviewed, unverified checksum, untrusted publisher key) all still fire exactly as they do from a card click. An unknown slug says so instead of failing silently. Deliberately reads the param on /extensions and not /marketplace: the old path resolves through a , which drops the query string, so a link to /marketplace?install=x would arrive with nothing to act on. Co-Authored-By: Claude Opus 5 (1M context) --- frontend/src/pages/Marketplace.jsx | 36 +++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/frontend/src/pages/Marketplace.jsx b/frontend/src/pages/Marketplace.jsx index 798570d1..c702e1c6 100644 --- a/frontend/src/pages/Marketplace.jsx +++ b/frontend/src/pages/Marketplace.jsx @@ -220,7 +220,7 @@ const Marketplace = () => { const [filters, setFilters] = useState({ ownership: '', category: [] }); const [filtersOpen, setFiltersOpen] = useState(false); const location = useLocation(); - const [searchParams] = useSearchParams(); + const [searchParams, setSearchParams] = useSearchParams(); const navigate = useNavigate(); // The active view is driven by the route (/marketplace = browse, // /marketplace/installed = installed). The legacy ?tab=installed deep link @@ -268,6 +268,40 @@ const Marketplace = () => { useEffect(() => { loadExtensions(); }, [loadExtensions]); + // Deep link: /extensions?install= opens that extension's detail + // modal. This is what a serverkit.ai install link lands on (and the + // counterpart to Templates.jsx's ?install=). + // + // It opens the modal rather than installing: a URL arriving from another + // site must never be able to install anything on its own, so the operator + // still presses Install and every trust gate behind it still fires. + // + // Note /marketplace?install=… cannot work — that path only redirects here + // via , which drops the query string. Links must target + // /extensions directly. + const installSlug = searchParams.get('install'); + useEffect(() => { + if (!installSlug || loading) return; + + // Lookup only, so the catalog's featured/sort ordering is irrelevant. + const entry = [ + ...builtins.map(getLocalCatalogEntry), + ...registryExtensions.map(getRegistryCatalogEntry), + ].find((candidate) => candidate.installKey === installSlug); + + if (entry) { + setDetailEntry(entry); + } else { + toast.error(`No extension named "${installSlug}" in this panel's catalog.`); + } + + // Drop the param either way so a refresh doesn't reopen it. + const next = new URLSearchParams(searchParams); + next.delete('install'); + setSearchParams(next, { replace: true }); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [installSlug, loading, builtins, registryExtensions]); + const handleBuiltinInstall = async (slug) => { setInstalling(true); try { From c3d70ed18b6e028971c1a38a290403467b2d61ed Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Thu, 6 Aug 2026 00:33:46 -0400 Subject: [PATCH 3/9] fix(templates): point the default repo at a host that exists, and verify what it sends DEFAULT_REPOS pointed at raw.githubusercontent.com/serverkit/templates -- the org `serverkit` does not exist (the registry is jhd3197/serverkit-templates, and its default branch is master, not main). So it has 404'd for its entire existence: no panel has ever fetched a template remotely, and every /templates page has been showing only the 118 bundled backend/templates/*.yaml. Now points at https://serverkit.ai/templates, which proxies the registry and was built for this consumer -- it serves /index.json and the /templates/.yaml path this class derives, behind a TTL cache with last-good fallback. Using the product domain also means a branch rename upstream cannot silently empty every panel's catalog, which is the exact failure this is fixing. Fixing the default alone would only help fresh installs, because get_config returns whatever a saved templates.json holds. Known-dead URLs are therefore healed on read. Nothing is lost by rewriting a URL that never once resolved. The checksum half is here rather than in a follow-up because this commit ACTIVATES a download path that has never run: sync_templates wrote whatever came back straight to disk, ignoring the sha256 the index pins for every one of the 106 official entries. Turning on an unverified fetch days after the panel learned to verify ed25519 signatures on extensions would be shipping a known downgrade. Mismatch is now a hard refusal that never reaches disk; absent hash is allowed and counted, mirroring unsigned-vs-invalid for extensions. Content is written as bytes so the file on disk is exactly what was hashed. Note get_template's in-memory remote read (it parses without saving) is not covered -- it has no index entry on hand and would need an extra fetch. DEPLOY ORDER: serverkit.ai must ship the master-branch fix before this helps. Until then the proxy 502s and sync finds nothing, same as today. Co-Authored-By: Claude Opus 5 (1M context) --- backend/app/services/template_service.py | 76 +++++++- backend/tests/test_template_repo_sync.py | 212 +++++++++++++++++++++++ 2 files changed, 281 insertions(+), 7 deletions(-) create mode 100644 backend/tests/test_template_repo_sync.py diff --git a/backend/app/services/template_service.py b/backend/app/services/template_service.py index d046d06b..22cb9b50 100644 --- a/backend/app/services/template_service.py +++ b/backend/app/services/template_service.py @@ -36,15 +36,32 @@ class TemplateService: INSTALLED_DIR = paths.APPS_DIR TEMPLATE_CONFIG = os.path.join(CONFIG_DIR, 'templates.json') - # Default template repository + # Default template repository. + # + # serverkit.ai proxies the serverkit-templates registry and is built for + # exactly this consumer: it serves /index.json and the + # /templates/.yaml path this class derives below, behind a + # TTL cache with last-good fallback. Pointing at the product domain rather + # than raw.githubusercontent also means a branch rename upstream cannot + # silently empty every panel's catalog. DEFAULT_REPOS = [ { 'name': 'serverkit-official', - 'url': 'https://raw.githubusercontent.com/serverkit/templates/main', + 'url': 'https://serverkit.ai/templates', 'enabled': True } ] + # Repo URLs that never worked and should be healed on read rather than + # left to rot in an operator's templates.json. `serverkit/templates` was a + # guess at the org name -- the registry is `jhd3197/serverkit-templates` -- + # so this URL has 404'd for its entire existence and no panel has ever + # fetched a template through it. Nothing is lost by replacing it. + DEAD_REPO_URLS = { + 'https://raw.githubusercontent.com/serverkit/templates/main', + 'https://raw.githubusercontent.com/serverkit/templates', + } + # Provider-owned templates (plan 52 D4 hook inversion): these ids are # listed and installable ONLY while the owning extension has registered as # their provider (i.e. it is installed + active this boot — registration @@ -167,11 +184,18 @@ def _run_provider_validate(cls, template_id, variables): @classmethod def get_config(cls) -> Dict: - """Get template configuration.""" + """Get template configuration. + + A panel that has ever saved this file keeps whatever repos were in it, + so fixing DEFAULT_REPOS alone would only help fresh installs. Dead URLs + are therefore corrected on read (see DEAD_REPO_URLS). Not written back + here -- a getter should not have a disk side effect -- so the repair + re-applies each read until something saves the config normally.""" if os.path.exists(cls.TEMPLATE_CONFIG): try: with open(cls.TEMPLATE_CONFIG, 'r') as f: - return json.load(f) + config = json.load(f) + return cls._heal_dead_repos(config) except Exception: pass return { @@ -180,6 +204,18 @@ def get_config(cls) -> Dict: 'last_sync': None } + @classmethod + def _heal_dead_repos(cls, config: Dict) -> Dict: + """Point any known-dead repo URL at the current default.""" + repos = config.get('repos') + if not isinstance(repos, list): + return config + default_url = cls.DEFAULT_REPOS[0]['url'] + for repo in repos: + if isinstance(repo, dict) and repo.get('url', '').rstrip('/') in cls.DEAD_REPO_URLS: + repo['url'] = default_url + return config + @classmethod def save_config(cls, config: Dict) -> Dict: """Save template configuration.""" @@ -2306,6 +2342,7 @@ def sync_templates(cls) -> Dict: config = cls.get_config() synced = 0 + unverified = 0 # saved, but the index pinned no sha256 for them errors = [] for repo in config.get('repos', []): @@ -2331,10 +2368,34 @@ def sync_templates(cls) -> Dict: response = requests.get(template_url, timeout=30) response.raise_for_status() - # Save locally + # Verify against the checksum the index pinned for this + # entry. A template is a deploy definition -- images, + # ports, volumes, env -- so a swapped file is worth + # refusing outright, and the index already carries the + # hash for every official entry. + # + # Missing hash is allowed (third-party repos may not + # publish one) and counted, mirroring how extensions + # treat unsigned-vs-invalid: absent is a caveat, wrong + # is a hard stop. + expected = (template_info.get('sha256') or '').strip().lower() + if expected: + actual = hashlib.sha256(response.content).hexdigest() + if actual != expected: + errors.append( + f"Checksum mismatch for {template_id}: index pinned " + f"{expected[:12]}..., downloaded {actual[:12]}.... Not saved." + ) + continue + else: + unverified += 1 + + # Written as bytes so what lands on disk is exactly the + # content that was hashed (text mode would rewrite line + # endings on Windows and no longer match). filepath = os.path.join(cls.TEMPLATES_DIR, f"{template_id}.yaml") - with open(filepath, 'w') as f: - f.write(response.text) + with open(filepath, 'wb') as f: + f.write(response.content) synced += 1 except Exception as e: @@ -2349,6 +2410,7 @@ def sync_templates(cls) -> Dict: return { 'success': True, 'synced': synced, + 'unverified': unverified, 'errors': errors if errors else None } diff --git a/backend/tests/test_template_repo_sync.py b/backend/tests/test_template_repo_sync.py new file mode 100644 index 00000000..ed4a8118 --- /dev/null +++ b/backend/tests/test_template_repo_sync.py @@ -0,0 +1,212 @@ +"""Proving tests for the template repository default and sync verification. + +Two things land here together on purpose. The default repo URL pointed at +`serverkit/templates`, an org that does not exist, so it 404'd for its whole +life and no panel ever fetched a template through it. Correcting it turns on +a download path that has therefore never actually run in production -- and +that path wrote whatever came back straight to disk without checking the +sha256 the index pins for every entry. Fixing the URL without the checksum +would be switching on an unverified fetch. +""" +import hashlib +import json +import os +from unittest.mock import patch + +import pytest + +from app.services.template_service import TemplateService + + +TEMPLATE_BODY = b"name: Demo\nversion: '1.0'\nservices:\n web:\n image: demo:1\n" +TEMPLATE_SHA = hashlib.sha256(TEMPLATE_BODY).hexdigest() + + +class FakeResponse: + def __init__(self, content=b"", payload=None, status_code=200): + self.content = content + self._payload = payload + self.status_code = status_code + + @property + def text(self): + return self.content.decode("utf-8") + + def json(self): + return self._payload + + def raise_for_status(self): + if self.status_code >= 400: + raise RuntimeError(f"HTTP {self.status_code}") + + +# -------------------------------------------------------------------------- +# Default repo + healing +# -------------------------------------------------------------------------- + +def test_default_repo_points_at_a_reachable_host(): + """The org `serverkit` does not exist; the registry is under jhd3197 and is + proxied by serverkit.ai. Guard against the old value coming back.""" + url = TemplateService.DEFAULT_REPOS[0]['url'] + assert 'raw.githubusercontent.com/serverkit/' not in url + assert url == 'https://serverkit.ai/templates' + + +def test_derived_urls_match_what_the_proxy_serves(): + """This class builds /index.json and + /templates/.yaml. serverkit.ai exposes both shapes; if this + ever drifts, every sync 404s silently.""" + base = TemplateService.DEFAULT_REPOS[0]['url'] + assert f"{base}/index.json" == 'https://serverkit.ai/templates/index.json' + assert f"{base}/templates/n8n.yaml" == 'https://serverkit.ai/templates/templates/n8n.yaml' + + +def test_dead_repo_url_is_healed_on_read(tmp_path): + """A panel that already saved templates.json keeps its repos, so fixing the + default alone would strand every existing install on the dead URL.""" + config_path = tmp_path / 'templates.json' + config_path.write_text(json.dumps({ + 'repos': [{ + 'name': 'serverkit-official', + 'url': 'https://raw.githubusercontent.com/serverkit/templates/main', + 'enabled': True, + }], + 'installed': {}, + 'last_sync': None, + })) + + with patch.object(TemplateService, 'TEMPLATE_CONFIG', str(config_path)): + config = TemplateService.get_config() + + assert config['repos'][0]['url'] == 'https://serverkit.ai/templates' + assert config['repos'][0]['name'] == 'serverkit-official' # nothing else touched + + +def test_healing_leaves_operator_repos_alone(tmp_path): + """Only the known-dead URLs are rewritten -- a custom repo is untouched.""" + config_path = tmp_path / 'templates.json' + config_path.write_text(json.dumps({ + 'repos': [ + {'name': 'mine', 'url': 'https://templates.example.com/sk', 'enabled': True}, + {'name': 'dead', 'url': 'https://raw.githubusercontent.com/serverkit/templates/main', + 'enabled': False}, + ], + })) + + with patch.object(TemplateService, 'TEMPLATE_CONFIG', str(config_path)): + repos = TemplateService.get_config()['repos'] + + assert repos[0]['url'] == 'https://templates.example.com/sk' + assert repos[1]['url'] == 'https://serverkit.ai/templates' + assert repos[1]['enabled'] is False # healing must not re-enable anything + + +def test_healing_survives_a_malformed_config(tmp_path): + """Garbage in the repos key must not take the whole panel's config down.""" + config_path = tmp_path / 'templates.json' + config_path.write_text(json.dumps({'repos': 'not-a-list'})) + + with patch.object(TemplateService, 'TEMPLATE_CONFIG', str(config_path)): + config = TemplateService.get_config() + + assert config['repos'] == 'not-a-list' # returned as-is, no crash + + +# -------------------------------------------------------------------------- +# sync_templates checksum verification +# -------------------------------------------------------------------------- + +def _run_sync(tmp_path, index_entry, body): + """Drive sync_templates against a one-entry fake repo.""" + index = {'templates': [index_entry]} + + def fake_get(url, timeout=None): + if url.endswith('/index.json'): + return FakeResponse(payload=index) + return FakeResponse(content=body) + + templates_dir = tmp_path / 'templates' + config_path = tmp_path / 'templates.json' + config_path.write_text(json.dumps({ + 'repos': [{'name': 'test', 'url': 'https://example.test/repo', 'enabled': True}], + })) + + with patch.object(TemplateService, 'TEMPLATES_DIR', str(templates_dir)), \ + patch.object(TemplateService, 'TEMPLATE_CONFIG', str(config_path)), \ + patch.object(TemplateService, 'CONFIG_DIR', str(tmp_path)), \ + patch('app.services.template_service.requests.get', side_effect=fake_get): + result = TemplateService.sync_templates() + + return result, templates_dir / 'demo.yaml' + + +def test_matching_checksum_is_saved(tmp_path): + result, path = _run_sync( + tmp_path, {'id': 'demo', 'sha256': TEMPLATE_SHA}, TEMPLATE_BODY) + + assert result['synced'] == 1 + assert result['unverified'] == 0 + assert not result['errors'] + assert path.exists() + # Byte-identical to what was verified. + assert hashlib.sha256(path.read_bytes()).hexdigest() == TEMPLATE_SHA + + +def test_mismatched_checksum_is_refused_and_not_written(tmp_path): + """A template is a deploy definition -- images, ports, volumes, env. A + swapped file is refused outright rather than saved with a warning.""" + result, path = _run_sync( + tmp_path, {'id': 'demo', 'sha256': 'de' * 32}, TEMPLATE_BODY) + + assert result['synced'] == 0 + assert not path.exists(), 'refused content must never reach disk' + assert result['errors'] and 'Checksum mismatch' in result['errors'][0] + + +def test_missing_checksum_is_allowed_but_counted(tmp_path): + """Third-party repos may publish no hashes; absent is a caveat, not a + hard stop -- mirroring unsigned-vs-invalid for extensions.""" + result, path = _run_sync(tmp_path, {'id': 'demo'}, TEMPLATE_BODY) + + assert result['synced'] == 1 + assert result['unverified'] == 1 + assert path.exists() + + +def test_checksum_comparison_ignores_case_and_padding(tmp_path): + result, path = _run_sync( + tmp_path, {'id': 'demo', 'sha256': f' {TEMPLATE_SHA.upper()} '}, TEMPLATE_BODY) + + assert result['synced'] == 1 + assert path.exists() + + +def test_one_bad_template_does_not_abort_the_rest(tmp_path): + """A poisoned entry must not stop the good ones from syncing.""" + good = b"name: Good\n" + index = {'templates': [ + {'id': 'bad', 'sha256': 'de' * 32}, + {'id': 'good', 'sha256': hashlib.sha256(good).hexdigest()}, + ]} + + def fake_get(url, timeout=None): + if url.endswith('/index.json'): + return FakeResponse(payload=index) + return FakeResponse(content=TEMPLATE_BODY if '/bad.yaml' in url else good) + + templates_dir = tmp_path / 'templates' + config_path = tmp_path / 'templates.json' + config_path.write_text(json.dumps({ + 'repos': [{'name': 'test', 'url': 'https://example.test/repo', 'enabled': True}], + })) + + with patch.object(TemplateService, 'TEMPLATES_DIR', str(templates_dir)), \ + patch.object(TemplateService, 'TEMPLATE_CONFIG', str(config_path)), \ + patch.object(TemplateService, 'CONFIG_DIR', str(tmp_path)), \ + patch('app.services.template_service.requests.get', side_effect=fake_get): + result = TemplateService.sync_templates() + + assert result['synced'] == 1 + assert (templates_dir / 'good.yaml').exists() + assert not (templates_dir / 'bad.yaml').exists() + assert len(result['errors']) == 1 From 8000faad0b6e923aba2c2d19e30e3581f614f91e Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Thu, 6 Aug 2026 00:38:27 -0400 Subject: [PATCH 4/9] harden(auth): close three gaps in the post-login redirect validator Reasoning about a redirect validator is how they ship broken, so this was driven by a probe that asks the URL parser directly: for ~50 candidate strings, does `new URL(candidate, base)` leave the origin, and does sanitizeRedirect agree? Zero leaks before or after -- the origin property was never broken. What the probe did surface was three ways to be wrong that are not origin escapes: 1. Dot segments were accepted. `/..//evil.com` normalizes to `//evil.com`, which stays same-origin only because it is resolved against a base; assigned to window.location.href it leaves the site. Rather than audit every present and future consumer of the stored value, the gadget is now refused outright -- plain and percent-encoded, in any position. A dot inside a segment name (/files/.env) is untouched. 2. Auth routes were matched as exact strings, so /login/ and, worse, /login/callback/ got through -- the latter re-runs the SSO callback with no code and lands the user on an error page right after a successful login. Now a prefix match on a "/" boundary, case-insensitive, which still leaves /logins and /login-help as valid destinations. 3. No expiry. A destination parked early in a tab session fired on any later login in that tab: park an install link, abandon it, log in normally half an hour later, get taken somewhere you had forgotten about. Now stamped and good for 30 minutes -- long enough for a password manager, a TOTP prompt, or an SSO round trip that includes signing up at the provider. Also a 2048-char cap, and consume now refuses a missing, non-numeric or future timestamp rather than trusting it: rememberRedirect is the only writer, so anything else did not come from this module. Tests go 9 -> 19, and now cover the remember/consume round trip (single use, expiry, tampered storage, storage unavailable) against a fake sessionStorage so the whole module still runs under plain node. Co-Authored-By: Claude Opus 5 (1M context) --- .../__tests__/redirectAfterLogin.test.mjs | Bin 3580 -> 8976 bytes frontend/src/utils/redirectAfterLogin.js | 56 ++++++++++++++++-- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/frontend/src/utils/__tests__/redirectAfterLogin.test.mjs b/frontend/src/utils/__tests__/redirectAfterLogin.test.mjs index 8de936259853289f9421945118706e14722fbfc2..41e5e72c71abfc6ce69ef9f361445a183e03a6b2 100644 GIT binary patch literal 8976 zcmcIq|8m>L5$@l5ikop#k}V0ya*}4~$R1CcX53Ni)N=o-I@S?55|F?<`0hYT)o3OU zktghv^xHcC1V~6EY?q^nN#Jg8@7v$I2}Aa)R;yGlm=(sdxzfxgg5}ECNv0O5WLm_j z7LjE!7NnBfR7tktSsHVzbZ``g_+y64)TaLt@2b_Q@q`L3Si-Y8Q}deCtGN~WjHr*9 z$+OflRaj;e!#7<>Lj(&M3(fM3g9WK#!G?w}1v3fH1>>GL*4mUF#@9b{D?+?H`Sv?^ z6Eh0fYs)+rL{G`=797z{jpZ_?FEe4+LP*B(Na3FNN7Hk>#xu!R>B3zXTq<`PEv|qB zk&CzyhH+fZ!<9-qWgf6)0U)KaARj4k$0buaPN{$#JNme4wUFq|R3Rh5W<0uPmVhuK zn^)kzE9BIJFqt?p86Aa%O*0dI_&^WLhYw*zegkemuY%>3IZBtg(zX<_R!cS@NT>9E z@Vt7=&L7N6+1>MW$vvd!M8xSf?^pYb^1bXk(kZY2yvOT?qTyish6I zuSe|k1smdoAmN67zNF&9o`?o)S9|gZ}Pzq(=YH09EXcImQV_|uk z`Q5uuaaB-kU6$HJ`u zr^C0^Y;}STyXdP{v5L$S16K^q@*-|OIZU-WFYv^w+>?CdbxVDGP%sE^bCX)pjFa0s z0{4b+YgMGOi59>-U5P21X|;w17}BvAk-#PtETNCaMLLdHk{}i;og!iePLliVAQY=K z3&ahFWJQ25L|1aI<}he>_HS8o4^kHwpT|pLZEmLF4oCfNL{vVT$Q5646Cw84DbElq zhfhcSxIoIp5p{2DnAl}D{=pM_(IC>WkV>z(y!Cktw&Eq#kG`3o0EK0cM5pu+B``M@V8wM|&UpV}B{%#kxj#YiPx{ZHm z+>au84}SH9JX7$w@Mu%!l)XW#Ln}!5@XQG`pn&Thvvrb230tcoi;;-JiS>)rE_ZDl zc?kPjED#8V?zs(#OyPWN4~&=TLL(scJPDTqI$N9Ig#STtKBT}?C*jO2i^T$g9_hNn zm9ibAJV*}->L?M>^`|#UAr~g%Q$eDpkTC@)s^1 zftc4kEhb3G9c1U{Z&4=sg>0PyO{tJ@qm*+8CCI2bn}Y)?5p(4DJ>U(uWV+AL9Gn}r z7fM0YJ!v?#?R#*kx2T~+9-eHS2-JcgAT+3)WU)d?Nn%5Bz;0FHwJ)`Y;d1u&=httx z4%i3F8P(w7kCilcU}^Xr)BHbLN73M4O-Cv$C$7j0!VPIsv6Mz0{t*gTb@rz3JO zPZmWQI{^*C)`P>+7botrx&^TBNj^`ZS91S-eo+8M9~#c zR7Gv)d=ASkT%Zh>A%d(=bda)LLKieDVRTi{?ofEUc@?mC!l-OTM!!Id1YU}xd4MBH zK$S_>VxZQ^#OIG=HbbukijgiPc7v|3ZiB3hK0yK(LY)}wCS^n!RM9JI@CA=aL-8If z3j-=}&6J_yY-)2*p+tasPK2eswqCgFuU1a6A0=`1p8qf7z!{x(sh#emreadDcb6J9=k$j@e0ya0^4p z*gxN$ZM-GGM)#%au?{1&r67ir<8QtnbwF=#bAdW>C&TYx>S@i?z$XLLPiZ*!s??B5 zPReY-1!*(}Y%2PpI_RaIE(PpSno?Vsf=b*+Oy8XJS$%iES?w@_^d?{KLp{!_SsIp} zk&<|10o2x{*I(|iN!8=p9Du{A;a9fjxxOZU)KEK@Gs378`q5hI8U;jNc+ac9>)*Pg}M|RAXFirnrtPH;cBPt2XpC((x_S ziD=cM{$vS&W{o?(x3JxT7QNuWAnh2l)(Y>FrXHVIm*muYH4@CxV!cOQIF$`*JL}2uzzs*Q#uxU#F z{rv5lzz2(Tep?!R+t_OZzX6OK1F|mG?nltE-Nf)gC3{R;;Q~CYF-<57m&N!(B+B4* zpl|KChx!vDNLq3!(;+g`a_$46tk9_44ugl;3S#SpqTpf?S2$Znui`j&DW&;|8hz|;>C2*vMW>(53AeY zil(e&S2VFj%298AP%oQ)ft0ZU8Y2Gw{gprXl)bH*#@j;Q<2Suciil&vspEr?hAszqlhncwQhM5suS;S94jq*RQo4t-VFycl=u{LN6%R~0`@@E6X(9$ qHY4XgJKN*uQVP`jdZX-n;&}*am@w}N&n?A$Ah#(6bbzEwNB2Lz2woxp delta 29 lcmbQ>_D6by2h-$iCe_VCEK4~jYYQBjJdsm)GryQ9BLJRJ2|@q> diff --git a/frontend/src/utils/redirectAfterLogin.js b/frontend/src/utils/redirectAfterLogin.js index c4cd7ceb..79e7910a 100644 --- a/frontend/src/utils/redirectAfterLogin.js +++ b/frontend/src/utils/redirectAfterLogin.js @@ -15,8 +15,17 @@ const STORAGE_KEY = 'serverkit.redirectAfterLogin'; +// How long a parked destination stays good. Long enough to cover a password +// manager, a TOTP prompt, or an SSO round trip that includes signing up at the +// provider; short enough that a destination abandoned earlier in the tab +// session does not resurface on an unrelated login and read as a glitch. +const MAX_AGE_MS = 30 * 60 * 1000; + +// A path far longer than any real panel route is not a destination. +const MAX_PATH_LENGTH = 2048; + // Landing back on one of these after login is either a loop or nonsense. -const AUTH_PATHS = ['/login', '/register', '/setup', '/migrate', '/logout']; +const AUTH_PATH_PREFIXES = ['/login', '/register', '/setup', '/migrate', '/logout']; /** * Validate a stored destination before we navigate to it. @@ -28,6 +37,7 @@ const AUTH_PATHS = ['/login', '/register', '/setup', '/migrate', '/logout']; */ export function sanitizeRedirect(path) { if (typeof path !== 'string' || !path) return null; + if (path.length > MAX_PATH_LENGTH) return null; if (path[0] !== '/') return null; if (path[1] === '/' || path[1] === '\\') return null; // Control characters (newline, tab, NUL) can smuggle a value past the @@ -36,7 +46,23 @@ export function sanitizeRedirect(path) { if (/[\u0000-\u001f\u007f]/.test(path)) return null; const pathname = path.split(/[?#]/)[0]; - if (AUTH_PATHS.includes(pathname)) return null; + + // Dot segments, plain or percent-encoded, in any position. They collapse: + // `/..//evil.com` normalizes to `//evil.com`, which is only same-origin + // while it is resolved against a base — hand that same string to + // `window.location.href` and it leaves the site. Nothing in this app needs + // a dot segment, so refuse the gadget rather than reason about every + // consumer of the value. + if (pathname.split('/').some((segment) => segment === '.' || segment === '..')) return null; + if (/%2e/i.test(pathname)) return null; + + // Prefix match with a "/" boundary, case-insensitive: catches /login/ and + // /login/callback/ (which would re-run the SSO callback with no + // code and show an error), while leaving /logins and /login-help valid. + const lowered = pathname.toLowerCase(); + if (AUTH_PATH_PREFIXES.some((p) => lowered === p || lowered.startsWith(`${p}/`))) { + return null; + } return path; } @@ -49,7 +75,7 @@ export function rememberRedirect(location) { ); if (!path || path === '/') return; // the dashboard is already the default try { - window.sessionStorage.setItem(STORAGE_KEY, path); + window.sessionStorage.setItem(STORAGE_KEY, JSON.stringify({ path, at: Date.now() })); } catch { // Storage unavailable — fall back to the dashboard, as before. } @@ -61,12 +87,30 @@ export function rememberRedirect(location) { * post-login navigation is not the place to trust that assumption. */ export function consumeRedirect() { - let stored = null; + let raw = null; try { - stored = window.sessionStorage.getItem(STORAGE_KEY); + raw = window.sessionStorage.getItem(STORAGE_KEY); window.sessionStorage.removeItem(STORAGE_KEY); } catch { return '/'; } - return sanitizeRedirect(stored) || '/'; + if (!raw) return '/'; + + let stored; + try { + stored = JSON.parse(raw); + } catch { + return '/'; + } + if (!stored || typeof stored !== 'object') return '/'; + + // A missing, non-numeric, or future timestamp is treated as expired rather + // than trusted: rememberRedirect is the only writer, so anything else is + // not a value this module put there. + const at = Number(stored.at); + if (!Number.isFinite(at) || at > Date.now() || Date.now() - at > MAX_AGE_MS) { + return '/'; + } + + return sanitizeRedirect(stored.path) || '/'; } From 4ccfa7e8cf14d74923bd50cad6b2321af1a7fd3e Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Thu, 6 Aug 2026 00:39:21 -0400 Subject: [PATCH 5/9] test: ratchet 3138 -> 3173 10 from the template repo/sync suite added alongside the DEFAULT_REPOS fix; the rest accumulated since the last ratchet. Co-Authored-By: Claude Opus 5 (1M context) --- backend/tests/BASELINE_COUNT | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/tests/BASELINE_COUNT b/backend/tests/BASELINE_COUNT index a0d2c03e..26807db2 100644 --- a/backend/tests/BASELINE_COUNT +++ b/backend/tests/BASELINE_COUNT @@ -1 +1 @@ -3138 \ No newline at end of file +3173 \ No newline at end of file From 44f5a328679bf62109142ef3e4d0dc3176a75bbd Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Thu, 6 Aug 2026 01:17:20 -0400 Subject: [PATCH 6/9] ci: stop validating the same commit twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These five workflows triggered on both `push: [dev, main]` and `pull_request: [dev, main]`, so one change was validated up to three times: on the dev push, again on the dev->main PR, and a third time on the post-merge push to main. Narrow them to `push: [dev]` + `pull_request: [main]`: - `pull_request: [dev]` was dead config. Every PR this repo has had targets main (checked back through #77, dependabot's included), so it never produced a run. - `push: [main]` was redundant for release-smoke: release.yml's build-release job runs that same scripts/build-release.sh for real, on the same commit, moments later. Also drops test-system-utils' `unit-tests` job. It ran `pytest tests/test_utils_system.py` (44 tests) that Backend CI's bare `pytest` already collects — there is no pytest.ini, addopts or collect_ignore narrowing collection. The distro matrix and the raw-subprocess audit stay; those are the parts Backend CI genuinely cannot do. Coverage of main is unchanged: a pull_request run tests the merge result, and release.yml still gates itself on the full backend suite. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/extensions-ci.yml | 4 +-- .github/workflows/release-smoke.yml | 7 ++++-- .github/workflows/scripts-ci.yml | 4 +-- .github/workflows/security-scan.yml | 2 +- .github/workflows/test-system-utils.yml | 33 +++++++------------------ 5 files changed, 19 insertions(+), 31 deletions(-) diff --git a/.github/workflows/extensions-ci.yml b/.github/workflows/extensions-ci.yml index 2b468270..1415c81f 100644 --- a/.github/workflows/extensions-ci.yml +++ b/.github/workflows/extensions-ci.yml @@ -8,14 +8,14 @@ name: Extensions CI on: push: - branches: [dev, main] + branches: [dev] paths: - 'builtin-extensions/**' - 'frontend/src/plugins/**' - 'scripts/sync-builtin-frontends.mjs' - '.github/workflows/extensions-ci.yml' pull_request: - branches: [dev, main] + branches: [main] paths: - 'builtin-extensions/**' - 'frontend/src/plugins/**' diff --git a/.github/workflows/release-smoke.yml b/.github/workflows/release-smoke.yml index b2a35cfb..b47ef848 100644 --- a/.github/workflows/release-smoke.yml +++ b/.github/workflows/release-smoke.yml @@ -5,9 +5,12 @@ name: Release Build Smoke Test # frontend source files) before they reach main, instead of failing late in the # release workflow. +# main is deliberately absent from `push`: on a merge to main, release.yml's +# `build-release` job runs this very script for real moments later, so building +# the tarball here too was pure duplication. on: push: - branches: [dev, main] + branches: [dev] paths: - 'frontend/**' - 'backend/**' @@ -16,7 +19,7 @@ on: - '.github/workflows/release.yml' - 'VERSION' pull_request: - branches: [dev, main] + branches: [main] paths: - 'frontend/**' - 'backend/**' diff --git a/.github/workflows/scripts-ci.yml b/.github/workflows/scripts-ci.yml index 24df61c2..4fe0dcd1 100644 --- a/.github/workflows/scripts-ci.yml +++ b/.github/workflows/scripts-ci.yml @@ -14,7 +14,7 @@ name: Scripts CI on: push: - branches: [dev, main] + branches: [dev] paths: - 'scripts/**' - 'templates/**' @@ -23,7 +23,7 @@ on: - 'serverkit' - '.github/workflows/scripts-ci.yml' pull_request: - branches: [dev, main] + branches: [main] paths: - 'scripts/**' - 'templates/**' diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index f565c33d..1861401c 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -7,7 +7,7 @@ on: - 'backend/**' - '.github/workflows/security-scan.yml' pull_request: - branches: [main, dev] + branches: [main] paths: - 'backend/**' - '.github/workflows/security-scan.yml' diff --git a/.github/workflows/test-system-utils.yml b/.github/workflows/test-system-utils.yml index 8dae39e2..35e02ba9 100644 --- a/.github/workflows/test-system-utils.yml +++ b/.github/workflows/test-system-utils.yml @@ -9,36 +9,21 @@ on: - 'backend/tests/test_utils_system*.py' - '.github/workflows/test-system-utils.yml' pull_request: - branches: [main, dev] + branches: [main] paths: - 'backend/app/utils/system.py' - 'backend/app/services/**' - 'backend/tests/test_utils_system*.py' +# The mocked `unit-tests` job that used to lead this file was removed: it ran +# `pytest tests/test_utils_system.py` (44 tests), and Backend CI's bare +# `pytest -v` already collects that exact file — there is no pytest.ini, +# addopts, or collect_ignore narrowing it. What is left here is the part +# Backend CI genuinely cannot do: exercise the package-manager detection +# against real apt/dnf inside real distro images. jobs: # ────────────────────────────────────────────────────────────────── - # Job 1: Mocked unit tests — fast, validates all logic - # ────────────────────────────────────────────────────────────────── - unit-tests: - name: Unit Tests - runs-on: ubuntu-latest - - steps: - - uses: actions/checkout@v4 - - - uses: actions/setup-python@v5 - with: - python-version: '3.11' - - - name: Install test deps - run: pip install pytest - - - name: Run unit tests - working-directory: backend - run: python -m pytest tests/test_utils_system.py -v - - # ────────────────────────────────────────────────────────────────── - # Job 2: Integration tests on real distros (no mocks) + # Job 1: Integration tests on real distros (no mocks) # ────────────────────────────────────────────────────────────────── integration-tests: name: Integration (${{ matrix.distro }}) @@ -91,7 +76,7 @@ jobs: run: python3 -m pytest tests/test_utils_system_integration.py -v # ────────────────────────────────────────────────────────────────── - # Job 3: Audit — grep for raw subprocess patterns in services + # Job 2: Audit — grep for raw subprocess patterns in services # ────────────────────────────────────────────────────────────────── audit-raw-patterns: name: Audit Raw Subprocess Patterns From d06c382364d7d32136bdabca15fe5ae6fb7269e0 Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Thu, 6 Aug 2026 01:17:56 -0400 Subject: [PATCH 7/9] ci: run the frontend lint gate, which ran nowhere MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `npm run lint` was defined in frontend/package.json and documented in CLAUDE.md, but no workflow ever invoked it. Grepping the whole workflow dir for `npm run lint`/`eslint`/`npm run build` returned nothing, so three project- specific checkers chained behind eslint were silently unenforced: check-settings-index every Settings tab has a search-index entry check-theme-tokens the theme-token whitelist stays in 3-way sync check-html-sinks every raw-HTML sink is sanitized or annotated (XSS) Lint currently passes: 926 warnings, 0 errors, and all three checkers green — so this is safe to add as a blocking check today. eslint exits 0 on warnings, so the gate is on errors only; add --max-warnings to freeze the count if the warning drift ever needs a ratchet of its own. Lint only, no build step: the frontend is already compiled in CI by Release Build Smoke Test, whose scripts/build-release.sh runs `npm ci && npm run build`. backend/app/** is in the paths because check-html-sinks scans it too, for `|safe`, `Markup(` and `render_template_string`. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/frontend-ci.yml | 58 +++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 .github/workflows/frontend-ci.yml diff --git a/.github/workflows/frontend-ci.yml b/.github/workflows/frontend-ci.yml new file mode 100644 index 00000000..e1f325fe --- /dev/null +++ b/.github/workflows/frontend-ci.yml @@ -0,0 +1,58 @@ +name: Frontend CI + +# The frontend's lint gate ran nowhere until now — `npm run lint` was in +# package.json and in CLAUDE.md, but no workflow ever invoked it. That silently +# unenforced three project-specific checkers that exist precisely because a +# human review keeps missing what they catch: +# +# check-settings-index every Settings tab has a search-index entry +# check-theme-tokens the theme-token whitelist stays in 3-way sync +# check-html-sinks every raw-HTML sink is sanitized or annotated (XSS) +# +# Only `npm run lint` runs here. The frontend is already COMPILED in CI by +# Release Build Smoke Test, whose scripts/build-release.sh does `npm ci && +# npm run build` — adding a build job here would just duplicate that. +# +# backend/app/** is in the paths because check-html-sinks scans it too (for +# `|safe`, `Markup(`, `render_template_string`), so a backend-only commit can +# introduce a sink this must catch. + +on: + push: + branches: [dev] + paths: + - 'frontend/**' + - 'backend/app/**' + - 'scripts/check-html-sinks.mjs' + - '.github/workflows/frontend-ci.yml' + pull_request: + branches: [main] + paths: + - 'frontend/**' + - 'backend/app/**' + - 'scripts/check-html-sinks.mjs' + - '.github/workflows/frontend-ci.yml' + workflow_call: + +jobs: + lint: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: '22' + cache: npm + cache-dependency-path: frontend/package-lock.json + - name: Install dependencies + working-directory: frontend + run: npm ci + - name: Lint + # eslint + the three checkers, chained by the package.json script. + # Currently 926 warnings / 0 errors, and eslint exits 0 on warnings — + # so this gates on errors only. If you ever want the warning count + # ratcheted the way backend/tests/BASELINE_COUNT ratchets test count, + # add --max-warnings= here rather than mass-fixing in one commit. + working-directory: frontend + run: npm run lint From ac48dc5b4e4a5ad3cdb5044a3cb20eb42c568704 Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Thu, 6 Aug 2026 01:18:26 -0400 Subject: [PATCH 8/9] perf(ci): shard the backend suite across 4 runners MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend CI was the entire pipeline wait. Measured on the last dev push: Extensions CI 12s Version Bump 18s Test System Utilities 35s Security Scan 41s Scripts CI 66s Release Build Smoke 68s Backend CI 21m 50s <- everything else finished in a minute Step timings put all of it in the test run itself (install deps 22s, ratchet 9s), so there is no caching win to take — it is 3173 tests against a function-scoped `app` fixture that rebuilds the Flask app and all 80+ tables per test. Fixing that fixture is the real repair and is planned separately; this change buys the wall-clock back now without touching a single test. Split over 4 runners via pytest-split (794/794/794/791). Sharding rather than pytest-xdist is deliberate: each shard is its own VM, so the process-shared state that makes in-process parallelism unsafe here (templates.json, APPS_DIR — see the per-PID DB dance in tests/conftest.py) simply isn't shared, and no test code has to change. The ratchet moves to its own job because it must see the whole suite; a shard only collects its quarter. It runs beside the shards and costs no wall-clock. Two details: - The shard command is scoped to `tests` rather than a bare `pytest`. Identical in CI (3173 either way, backend/dev-data/ being gitignored), but a bare pytest on a dev box tries to collect the locally deployed apps under backend/dev-data/ and dies during collection. The line is now copy-pasteable for debugging a red shard locally. - With no .test_durations file, pytest-split balances by test count, not time, and these tests range from 0.1s to 10s+. If one shard becomes the new critical path, run once with --store-durations and commit the file. main also drops out of `push` here: release.yml gates itself on this workflow via ci-gate, so listing main ran the whole suite a second time on every merge. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/backend-ci.yml | 61 ++++++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 6 deletions(-) diff --git a/.github/workflows/backend-ci.yml b/.github/workflows/backend-ci.yml index 34b1142f..83b79e60 100644 --- a/.github/workflows/backend-ci.yml +++ b/.github/workflows/backend-ci.yml @@ -5,22 +5,28 @@ name: Backend CI # registry, pairing, and the panel<->agent command loop exercised end-to-end # by tests/test_agent_poll_e2e.py — instead of finding out at install time. +# main is deliberately absent from `push`: release.yml gates itself on this +# workflow via `ci-gate`, so a push to main already runs the suite once. Listing +# main here ran it a second time, standalone, on every merge. on: push: - branches: [dev, main] + branches: [dev] paths: - 'backend/**' - '.github/workflows/backend-ci.yml' pull_request: - branches: [dev, main] + branches: [main] paths: - 'backend/**' - '.github/workflows/backend-ci.yml' workflow_call: jobs: - pytest: - name: pytest + # The ratchet needs to see the WHOLE suite, so it cannot live inside a shard — + # each shard only collects its own quarter. Its own job costs no wall-clock + # time: it runs beside the shards and finishes in seconds. + ratchet: + name: test-count ratchet runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 @@ -42,6 +48,49 @@ jobs: # Lowering the floor requires editing BASELINE_COUNT in the same commit. working-directory: backend run: python tests/check_test_count.py - - name: Run tests + + # Sharded because this job WAS the entire wait: 21m50s of a ~22m pipeline, + # while every other workflow finished inside a minute. Splitting the 3173 + # tests over 4 runners cuts the critical path to roughly a quarter. + # + # Sharding rather than pytest-xdist is deliberate: each shard is its own VM, + # so the process-shared state that makes in-process parallelism unsafe here + # (templates.json, APPS_DIR — see the per-PID DB dance in tests/conftest.py) + # simply isn't shared. No test code has to change. + # + # NOTE: with no .test_durations file, pytest-split balances by test COUNT, + # not by time — and these tests range from 0.1s to 10s+. If one shard lands + # most of the slow ones it becomes the new critical path. If that shows up, + # run once with `--store-durations` and commit backend/.test_durations to + # switch it to duration-based balancing. + pytest: + name: pytest (${{ matrix.group }}/4) + runs-on: ubuntu-latest + strategy: + # Report every failing shard in one pass instead of hiding shards 2-4 + # behind the first failure — one round of fixes instead of four. + fail-fast: false + matrix: + group: [1, 2, 3, 4] + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: '3.11' + cache: pip + cache-dependency-path: backend/requirements.txt + - name: Install dependencies + working-directory: backend + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt + pip install pytest pytest-split + - name: Run tests (shard ${{ matrix.group }} of 4) + # Scoped to `tests` rather than a bare `pytest`. Identical here (3173 + # either way, since backend/dev-data/ is gitignored and absent from a + # CI checkout), but it makes the command reproducible on a dev box: a + # bare pytest there tries to collect the locally deployed apps under + # backend/dev-data/ and dies during collection. Copy this line verbatim + # to debug a red shard locally. working-directory: backend - run: python -m pytest -v + run: python -m pytest tests -v --splits 4 --group ${{ matrix.group }} From 20effa2a1bf62738b11fde8d635d2a57aea1ad17 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 6 Aug 2026 05:34:26 +0000 Subject: [PATCH 9/9] chore: bump version to 1.7.83 [skip ci] --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index fba132c4..2fff9b2e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.7.82 +1.7.83