Skip to content

Make install links survive the login screen, and speed up CI - #92

Merged
jhd3197 merged 9 commits into
mainfrom
dev
Aug 6, 2026
Merged

Make install links survive the login screen, and speed up CI#92
jhd3197 merged 9 commits into
mainfrom
dev

Conversation

@jhd3197

@jhd3197 jhd3197 commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Four separate things in this branch turned out to be doing nothing at all, which is its own strange kind of relief. Install links from serverkit.ai — the entire point of the README badges — were discarded the moment login intervened, because PrivateRoute bounced to /login without recording where the visitor was going and every post-auth path then navigated to /; the /extensions?install=<slug> handler those links aim at had simply never been written, even though Templates.jsx has had its counterpart for a while. Meanwhile the default template repository pointed at a GitHub org that does not exist, so it has 404'd for its entire existence and no panel has ever fetched a template remotely, and npm run lint — defined in package.json, documented in CLAUDE.md — was invoked by no workflow, silently unenforcing three project-specific checkers including the raw-HTML sink audit. The redirect is parked in sessionStorage rather than react-router location state because the SSO flow leaves the origin entirely and comes back through /login/callback/<provider>, which router state cannot survive — one mechanism covering all four post-auth exits beats two each covering half — and it is validated on write and on read, since login is exactly where an open redirect hurts. The install deep link deliberately opens the extension's detail card instead of installing: a URL arriving from another site must not be able to install anything on its own, so the operator still presses Install and every trust gate behind it still fires. The CI half is separate housekeeping — five workflows were validating the same commit up to three times, and Backend CI's 21m50s was the pipeline wait while everything else finished inside a minute — so the suite now shards across four runners, chosen over pytest-xdist because each shard is its own VM and the process-shared state that makes in-process parallelism unsafe here simply isn't shared.

Highlights

  • Signing in now returns you to the page you originally asked for, instead of dumping you on the dashboard with no explanation of why the thing you clicked did not happen. This covers all four sign-in paths: password, two-factor, magic link, and SSO.
  • Install links from serverkit.ai finally land on the extension they name. The link opens that extension's detail card so you can review it and press Install yourself — a link from another site can never install anything on its own.
  • An install link naming an extension this panel does not know about now says so, rather than quietly doing nothing.
  • The panel can fetch templates from the registry for the first time. Every install to date has only ever seen the 118 templates bundled with the panel. This needs serverkit.ai's own fix deployed first; until then, syncing finds nothing, exactly as it does today.
  • Templates downloaded from a repository are checked against the checksum the registry pins for them, and a mismatched file is refused before it ever reaches disk.
  • Pipeline feedback arrives in roughly a quarter of the time, and pushes no longer burn three duplicate validation runs per change.
Technical changes

Post-login redirect

  • New frontend/src/utils/redirectAfterLogin.js exporting rememberRedirect(location), consumeRedirect(), and the pure sanitizeRedirect(path).
  • PrivateRoute in App.jsx calls rememberRedirect(location) before <Navigate to="/login" replace />; Login.jsx (link redemption, password, 2FA) and SSOCallback.jsx all navigate to consumeRedirect() instead of '/'.
  • Storage is sessionStorage under serverkit.redirectAfterLogin, holding {path, at}. Chosen over router state because the SSO round trip leaves the origin; a single mechanism then covers every post-auth exit.
  • sanitizeRedirect enforces same-origin by shape — one absolute path and nothing else. //evil.com and /\evil.com are rejected explicitly, since browsers read both as protocol-relative and either would turn login into an open redirect.
  • Control characters (newline, tab, NUL, DEL) are rejected: they can smuggle a value past the shape checks once something downstream re-parses it.
  • Dot segments are refused outright, plain and percent-encoded, in any position. /..//evil.com normalizes to //evil.com, which is only same-origin while resolved against a base — assigned to window.location.href it leaves the site. Refusing the gadget beats auditing every present and future consumer of the stored value. A dot inside a segment name (/files/.env) is untouched.
  • Auth-route rejection moved from exact-string to a case-insensitive prefix match on a / boundary, so /login/ and /login/callback/<provider> are caught — the latter would re-run the SSO callback with no code and land the user on an error page immediately after a successful login. /logins and /login-help stay valid.
  • Parked destinations expire after 30 minutes, sized for a password manager, a TOTP prompt, or an SSO round trip that includes signing up at the provider. Without it, a destination abandoned early in a tab session fired on any later login in that tab.
  • consumeRedirect treats a missing, non-numeric, or future at as expired rather than trusting it — rememberRedirect is the only writer, so anything else did not come from this module. Also a 2048-char path cap, single-use semantics (read clears), re-validation on read, and a try/catch fallback to / when storage is unavailable.

Extension install deep link

  • Marketplace.jsx reads ?install=<slug> and sets detailEntry to the matching catalog entry, searching builtins and registryExtensions by installKey so featured/sort ordering is irrelevant.
  • Opens the modal rather than installing, keeping the unreviewed / unverified-checksum / untrusted-publisher-key gates on the operator's click path exactly as a card click does. An unmatched slug raises a toast naming the slug.
  • The param is stripped via setSearchParams(next, {replace: true}) either way, so a refresh does not reopen it.
  • Reads on /extensions, not /marketplace — the latter resolves through a <Navigate>, which drops the query string.

Template repository

  • TemplateService.DEFAULT_REPOS moves from raw.githubusercontent.com/serverkit/templates/main (nonexistent org; the registry is jhd3197/serverkit-templates, default branch master) to https://serverkit.ai/templates, which proxies the registry, serves the <repo_url>/index.json and <repo_url>/templates/<id>.yaml paths this class derives, and caches with last-good fallback. The product domain also means an upstream branch rename cannot silently empty every panel's catalog — the exact failure being fixed.
  • New DEAD_REPO_URLS set plus _heal_dead_repos(), applied in get_config(). Fixing the default alone would only help fresh installs, since get_config returns whatever a saved templates.json holds. Not written back — a getter should not have a disk side effect — so the repair re-applies each read until the config is saved normally.
  • sync_templates() now verifies each download against the sha256 the index pins, comparing against response.content. Mismatch appends a truncated-hash error and continues without writing; a missing hash is permitted and counted in a new unverified field on the return value, mirroring the unsigned-vs-invalid split used for extensions.
  • Files are written in binary mode so what lands on disk is byte-identical to what was hashed — text mode would rewrite line endings on Windows and break the comparison.
  • Verification is in this commit rather than a follow-up because the URL fix activates a download path that has never once run; shipping an unverified fetch days after the panel learned to verify ed25519 signatures on extensions would be a known downgrade.
  • get_template's in-memory remote read (parses without saving) is not covered — it has no index entry on hand and would need an extra fetch.
  • Deploy ordering: serverkit.ai must ship its master-branch fix before this helps. Until then the proxy 502s and sync finds nothing, same as today.

CI

  • backend-ci.yml splits pytest into a 4-way matrix using pytest-split (--splits 4 --group ${{ matrix.group }}), roughly 794 tests per shard. Sharding over pytest-xdist because each shard is its own VM, so templates.json / APPS_DIR and the per-PID DB dance in tests/conftest.py are not shared and no test code changes.
  • fail-fast: false on the matrix, so all four shards report in one pass instead of hiding shards 2-4 behind the first failure.
  • The test-count ratchet becomes its own job — a shard only collects its quarter, so it cannot see the whole suite. It runs alongside the shards and costs no wall-clock.
  • Shard command is scoped to pytest tests rather than a bare pytest. Identical in CI (backend/dev-data/ is gitignored and absent), but a bare pytest on a dev box tries to collect locally deployed apps and dies during collection — the line is now copy-pasteable for debugging a red shard.
  • Known limitation, documented inline: 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.
  • New frontend-ci.yml running npm run lint (eslint plus check-settings-index, check-theme-tokens, check-html-sinks). Currently 926 warnings / 0 errors, and eslint exits 0 on warnings, so the gate is on errors only; --max-warnings=<N> is the documented lever if the warning count ever needs its own ratchet. backend/app/** is in the trigger paths because check-html-sinks scans it for |safe, Markup(, and render_template_string. Lint only — the frontend is already compiled by Release Build Smoke Test's scripts/build-release.sh.
  • Five workflows (backend-ci, extensions-ci, release-smoke, scripts-ci, security-scan) narrow from push: [dev, main] + pull_request: [dev, main] to push: [dev] + pull_request: [main]. pull_request: [dev] was dead config — every PR in this repo targets main, checked back through Test Sandbox, resumable LocalKit sync, and installer hardening #77 including dependabot's. push: [main] was redundant: release.yml gates itself on Backend CI via ci-gate and its build-release job runs scripts/build-release.sh for real moments later.
  • test-system-utils.yml drops its unit-tests job, which ran pytest tests/test_utils_system.py (44 tests) that Backend CI's collection already includes — there is no pytest.ini, addopts, or collect_ignore narrowing it. The real-distro integration matrix and the raw-subprocess audit stay; those are what Backend CI genuinely cannot do.
  • Coverage of main is unchanged: a pull_request run tests the merge result, and release.yml still gates on the full backend suite.
  • backend/tests/BASELINE_COUNT ratchets 3138 → 3173.

jhd3197 and others added 9 commits August 6, 2026 00:16
…hboard

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=<slug>, /templates?install=<id>) 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/<provider>; 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) <noreply@anthropic.com>
The counterpart to Templates.jsx's ?install=<id>, 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 <Navigate>, 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) <noreply@anthropic.com>
…ify 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 <repo_url>/index.json and the
<repo_url>/templates/<id>.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) <noreply@anthropic.com>
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/<provider> 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
`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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 6, 2026 05:48
@jhd3197
jhd3197 merged commit b2bba02 into main Aug 6, 2026
1 check passed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves deep-link usability and operational safety by preserving intended destinations across authentication (including SSO), adding an /extensions?install=<slug> deep link handler, fixing the default template repo URL (with on-read healing for existing installs), and speeding up CI via sharded backend tests plus an explicit frontend lint workflow.

Changes:

  • Preserve and safely restore post-login navigation targets via sessionStorage (covers password, 2FA, magic link, and SSO).
  • Support extension install deep links by opening the matching extension detail modal from /extensions?install=<slug>.
  • Fix template repo defaults + add checksum verification for synced templates; accelerate CI by sharding backend pytest and adding frontend lint enforcement.

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
VERSION Bumps panel version to 1.7.83.
frontend/src/utils/redirectAfterLogin.js New sessionStorage-based redirect capture/consume with sanitization to prevent open redirects/loops.
frontend/src/pages/SSOCallback.jsx Redirects to the stored destination after successful SSO login.
frontend/src/pages/Login.jsx Redirects to the stored destination after login/magic-link redemption/2FA completion.
frontend/src/App.jsx Updates PrivateRoute to remember the intended destination before redirecting to /login.
frontend/src/pages/Marketplace.jsx Handles /extensions?install=<slug> by opening the matching extension detail modal and then stripping the query param.
backend/app/services/template_service.py Fixes default template repo URL, heals dead URLs on read, and adds sha256 verification + unverified count during sync.
backend/tests/test_template_repo_sync.py Adds coverage for default repo URL, healing behavior, and checksum verification semantics.
backend/tests/BASELINE_COUNT Raises the test-count floor to 3173.
.github/workflows/backend-ci.yml Shards backend pytest into a 4-way matrix and moves the test-count ratchet into its own job.
.github/workflows/frontend-ci.yml Adds a dedicated lint workflow to enforce npm run lint (eslint + project checkers).
.github/workflows/extensions-ci.yml Adjusts triggers to reduce duplicate runs (push dev / PR main).
.github/workflows/release-smoke.yml Adjusts triggers and documents avoiding duplicate main-branch builds.
.github/workflows/scripts-ci.yml Adjusts triggers to reduce duplicate runs (push dev / PR main).
.github/workflows/security-scan.yml Adjusts PR triggers to main only.
.github/workflows/test-system-utils.yml Removes redundant mocked unit-tests job; retains real-distro integration matrix + audit job.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +214 to +216
for repo in repos:
if isinstance(repo, dict) and repo.get('url', '').rstrip('/') in cls.DEAD_REPO_URLS:
repo['url'] = default_url
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants