From 6447b704bb752684ff76936a90deb03c54b14815 Mon Sep 17 00:00:00 2001 From: skulidropek <66840575+skulidropek@users.noreply.github.com> Date: Thu, 18 Jun 2026 16:07:02 +0000 Subject: [PATCH] fix(api): stabilize skiller and browser launch - Route external Skiller launches through short backend records and active project filtering. - Repair browser sidecar startup so noVNC scales cleanly and Chromium opens about:blank without first-run/restore screens. - Proof of fix: packages/api browser/skiller tests passed, packages/app browser action tests passed, API build/lint passed, and live noVNC/CDP flow was verified. --- README.md | 13 ++ docker-compose.api.yml | 4 + docker-compose.yml | 4 + docs/integrations/skiller.md | 21 ++- packages/api/src/http.ts | 110 +++++++++++- .../api/src/services/project-browser-core.ts | 2 +- packages/api/src/services/project-browser.ts | 93 ++++++++++ packages/api/src/services/skiller-core.ts | 142 +++++++++++++++- packages/api/src/services/skiller.ts | 152 ++++++++++++++--- .../api/src/services/terminal-sessions.ts | 25 ++- packages/api/tests/project-browser.test.ts | 41 +++++ .../tests/project-port-forward-core.test.ts | 1 + packages/api/tests/skiller-core.test.ts | 81 +++++++++ packages/api/tests/skiller-cors.test.ts | 159 ++++++++++++++++++ packages/api/tests/skiller-routes.test.ts | 9 +- packages/api/tests/terminal-sessions.test.ts | 46 +++++ .../app/scripts/serve-dist-web-forwarding.mjs | 37 ++++ packages/app/scripts/serve-dist-web.mjs | 15 +- .../tests/docker-git/actions-skiller.test.ts | 32 ++++ .../controller-resource-limits.test.ts | 14 ++ .../tests/docker-git/serve-dist-web.test.ts | 19 +++ 21 files changed, 971 insertions(+), 49 deletions(-) create mode 100644 packages/app/scripts/serve-dist-web-forwarding.mjs diff --git a/README.md b/README.md index cda52047..fb602017 100644 --- a/README.md +++ b/README.md @@ -61,6 +61,19 @@ bun run docker-git -- browser DOCKER_GIT_WEB_HOST=127.0.0.1 bun run docker-git -- browser ``` +Кнопка `Skiller` в web-терминале по умолчанию открывает внешний Skiller Web: +`https://skiller-web-henna.vercel.app`. Контроллер сохраняет `backendUrl`, +`projectKey` и `sessionId` в коротком launch-wrapper URL вида +`/api/skiller/external-launch/`, чтобы длинный callback URL не оставался в +адресной строке. Если web-версия открыта по локальному HTTP/LAN адресу, +docker-git автоматически запускает или переиспользует Cloudflare Quick Tunnel +и передает в Skiller Web HTTPS `backendUrl` вида `https://*.trycloudflare.com/api`. +Для legacy bundled-режима: + +```bash +DOCKER_GIT_SKILLER_WEB_URL= bun run docker-git -- browser +``` + ## CLI пример Можно передавать ссылку на репозиторий, ветку (`/tree/...`), issue или PR. diff --git a/docker-compose.api.yml b/docker-compose.api.yml index 910129f8..41c85b20 100644 --- a/docker-compose.api.yml +++ b/docker-compose.api.yml @@ -27,6 +27,10 @@ services: DOCKER_GIT_EXCHANGE_AGENT_COMMAND: ${DOCKER_GIT_EXCHANGE_AGENT_COMMAND:-} DOCKER_GIT_EXCHANGE_AGENT_TIMEOUT_MS: ${DOCKER_GIT_EXCHANGE_AGENT_TIMEOUT_MS:-3600000} DOCKER_GIT_OUTBOX_POLLING_INTERVAL_MS: ${DOCKER_GIT_OUTBOX_POLLING_INTERVAL_MS:-5000} + DOCKER_GIT_SKILLER_WEB_URL: ${DOCKER_GIT_SKILLER_WEB_URL-https://skiller-web-henna.vercel.app} + DOCKER_GIT_SKILLER_BACKEND_URL: ${DOCKER_GIT_SKILLER_BACKEND_URL:-} + DOCKER_GIT_API_PUBLIC_URL: ${DOCKER_GIT_API_PUBLIC_URL:-} + DOCKER_GIT_SKILLER_ALLOWED_ORIGINS: ${DOCKER_GIT_SKILLER_ALLOWED_ORIGINS:-} ports: - "${DOCKER_GIT_API_BIND_HOST:-127.0.0.1}:${DOCKER_GIT_API_PORT:-3334}:${DOCKER_GIT_API_PORT:-3334}" dns: diff --git a/docker-compose.yml b/docker-compose.yml index 910129f8..41c85b20 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -27,6 +27,10 @@ services: DOCKER_GIT_EXCHANGE_AGENT_COMMAND: ${DOCKER_GIT_EXCHANGE_AGENT_COMMAND:-} DOCKER_GIT_EXCHANGE_AGENT_TIMEOUT_MS: ${DOCKER_GIT_EXCHANGE_AGENT_TIMEOUT_MS:-3600000} DOCKER_GIT_OUTBOX_POLLING_INTERVAL_MS: ${DOCKER_GIT_OUTBOX_POLLING_INTERVAL_MS:-5000} + DOCKER_GIT_SKILLER_WEB_URL: ${DOCKER_GIT_SKILLER_WEB_URL-https://skiller-web-henna.vercel.app} + DOCKER_GIT_SKILLER_BACKEND_URL: ${DOCKER_GIT_SKILLER_BACKEND_URL:-} + DOCKER_GIT_API_PUBLIC_URL: ${DOCKER_GIT_API_PUBLIC_URL:-} + DOCKER_GIT_SKILLER_ALLOWED_ORIGINS: ${DOCKER_GIT_SKILLER_ALLOWED_ORIGINS:-} ports: - "${DOCKER_GIT_API_BIND_HOST:-127.0.0.1}:${DOCKER_GIT_API_PORT:-3334}:${DOCKER_GIT_API_PORT:-3334}" dns: diff --git a/docs/integrations/skiller.md b/docs/integrations/skiller.md index b0c6345c..022711ab 100644 --- a/docs/integrations/skiller.md +++ b/docs/integrations/skiller.md @@ -40,15 +40,26 @@ bun run skiller:check ## docker-git Web Launch -The docker-git web terminal header includes a `Skiller` button next to `Open browser`. In a project terminal the button calls `POST /projects/by-key/:projectKey/terminal-sessions/:sessionId/skiller/open` first, which launches the pinned submodule Electron app as a separate process, registers the terminal session filesystem scope, and writes launcher output to `~/.docker-git/logs/skiller.log`. After that response succeeds, the browser opens the returned `/api/ssh/session/:sessionId/skiller/app/` URL using the same terminal session id that is present in `/ssh/session/:sessionId`. +The docker-git web terminal header includes a `Skiller` button next to `Open browser`. In a project terminal the button calls `POST /projects/by-key/:projectKey/terminal-sessions/:sessionId/skiller/open` first. By default the controller stores the external Skiller Web launch URL for `https://skiller-web-henna.vercel.app/launch` and returns a short docker-git wrapper path such as `/api/skiller/external-launch/`. The saved target carries `backendUrl`, `projectKey`, and `sessionId` parameters, while the browser address bar keeps the short wrapper URL. When docker-git web is served through the `/api` proxy, `backendUrl` includes that forwarded prefix so Skiller Web can call back to the same browser-reachable docker-git endpoint. -docker-git serves Skiller's built renderer from the submodule and proxies `/api/ssh/session/:sessionId/skiller/trpc/*` to the running Skiller tRPC backend, so the user sees the actual Skiller UI instead of an invisible background desktop process. The session id is part of the URL so a Skiller tab can be tied back to the terminal container that opened it. +Hosted Skiller Web is served over HTTPS, so browser fetches to a plain `http://192.168.x.x:4174/api` backend can be blocked before they reach docker-git. If external Skiller Web is enabled and no explicit backend override is configured, docker-git automatically starts or reuses the panel Cloudflare Quick Tunnel for local/private HTTP origins and sends Skiller Web an HTTPS callback URL such as `https://.trycloudflare.com/api`. Follow-up API calls through that tunnel preserve the HTTPS forwarded protocol so the controller does not try to create a nested tunnel. -For project terminals, docker-git scopes Skiller to the active project container filesystem. The API inspects the selected project container mounts, maps `/home/` and the project `targetDir` to the controller-visible Docker volume path, launches Skiller with `HOME` set to that mapped home directory, and registers the mapped project directory in Skiller. This makes global skill operations target the selected container home and project skill operations target the selected container project directory. If the controller cannot access the Docker volume path, the endpoint fails instead of opening Skiller against the wrong filesystem. +External Skiller Web is controlled by these API/controller environment variables: + +- `DOCKER_GIT_SKILLER_WEB_URL` sets the Skiller Web base URL. The compose default is `https://skiller-web-henna.vercel.app`. +- `DOCKER_GIT_SKILLER_BACKEND_URL` overrides the callback base URL sent to Skiller Web and disables automatic tunnel URL selection. +- `DOCKER_GIT_API_PUBLIC_URL` is the secondary callback override when `DOCKER_GIT_SKILLER_BACKEND_URL` is unset and also disables automatic tunnel URL selection. +- `DOCKER_GIT_SKILLER_ALLOWED_ORIGINS` adds comma-separated trusted Skiller Web origins for CORS. + +Set `DOCKER_GIT_SKILLER_WEB_URL=` to force the legacy bundled renderer flow. `DOCKER_GIT_CONTROLLER_BUILD_SKILLER=0` only controls whether the controller image bundles the Skiller submodule; it does not enable external Web mode by itself. + +In the legacy bundled mode, docker-git launches the pinned submodule Electron app as a separate process, registers the terminal session filesystem scope, writes launcher output to `~/.docker-git/logs/skiller.log`, serves Skiller's built renderer from the submodule, and proxies `/api/ssh/session/:sessionId/skiller/trpc/*` to the running Skiller tRPC backend. The session id is part of the URL so a Skiller tab can be tied back to the terminal container that opened it. + +For project terminals, docker-git scopes Skiller to the active project container filesystem. The API inspects the selected project container mounts, maps `/home/` and the project `targetDir` to the controller-visible Docker volume path, launches or reuses the local Skiller runtime with `HOME` set to that mapped home directory, and registers the mapped project directory in Skiller. This makes global skill operations target the selected container home and project skill operations target the selected container project directory. If the controller cannot access the Docker volume path, the endpoint fails instead of opening Skiller against the wrong filesystem. For Codex, Skiller resolves `~/.codex/skills` against the selected container home volume. For example, `/home/dev/.codex/skills` inside the selected container is exposed to the controller as the mapped Docker volume path and is the only Codex global skill tree used for that session. docker-git does not fall back to the controller's own `~/.codex/skills`. -When the API process has no `$DISPLAY`, the launcher uses `xvfb-run` if it is available so Skiller can still start in a headless controller environment. +When the API process has no `$DISPLAY`, the legacy bundled launcher uses `xvfb-run` if it is available so Skiller can still start in a headless controller environment. ## PR #238 Proof @@ -87,4 +98,4 @@ bun run skiller:check ## Integration Boundary -This integration makes Skiller part of the docker-git checkout and developer workflow. docker-git keeps Skiller as an isolated submodule and does not import Skiller source into the docker-git web bundle. The visible browser view is served from Skiller's own built renderer and backed by Skiller's own tRPC process. +This integration keeps the Skiller runtime and filesystem scope owned by docker-git while the default browser UI is served by external Skiller Web. The pinned submodule remains available for legacy bundled rendering and local development, but docker-git does not import Skiller source into the docker-git web bundle. diff --git a/packages/api/src/http.ts b/packages/api/src/http.ts index 0118efac..ebdfa495 100644 --- a/packages/api/src/http.ts +++ b/packages/api/src/http.ts @@ -6,6 +6,7 @@ import * as HttpRouter from "@effect/platform/HttpRouter" import * as HttpServerRequest from "@effect/platform/HttpServerRequest" import * as HttpServerResponse from "@effect/platform/HttpServerResponse" import * as HttpServerError from "@effect/platform/HttpServerError" +import { networkInterfaces } from "node:os" import * as ParseResult from "effect/ParseResult" import * as Schema from "effect/Schema" import { renderError, type AppError } from "@effect-template/lib/usecases/errors" @@ -168,13 +169,16 @@ import { openSkiller, openSkillerForTerminalSession, parseSkillerRoute, + inspectableSkillerConnectProjects, proxySkillerTrpc, + readExternalSkillerLaunchHtml, readSkillerProjectContext, serveSkillerApp } from "./services/skiller.js" import { isSkillerWebCorsOriginAllowed, - resolveDockerGitSkillerBackendUrl + joinSkillerBackendUrl, + resolveDockerGitSkillerBackendUrlDecision } from "./services/skiller-core.js" import { commitStateFromRequest, @@ -244,6 +248,10 @@ const SkillerConnectRequestSchema = Schema.Struct({ sessionId: Schema.optional(Schema.String) }) +const SkillerExternalLaunchParamsSchema = Schema.Struct({ + launchId: Schema.String +}) + type ApiError = | ApiAuthRequiredError | ApiBadRequestError @@ -448,6 +456,7 @@ const terminalSessionParams = HttpRouter.schemaParams(TerminalSessionParamsSchem const terminalSessionByProjectKeyParams = HttpRouter.schemaParams(TerminalSessionByProjectKeyParamsSchema) const containerTaskParams = HttpRouter.schemaParams(ContainerTaskParamsSchema) const authTerminalSessionParams = HttpRouter.schemaParams(AuthTerminalSessionParamsSchema) +const skillerExternalLaunchParams = HttpRouter.schemaParams(SkillerExternalLaunchParamsSchema) const readCreateProjectRequest = () => HttpServerRequest.schemaBodyJson(CreateProjectRequestSchema) const readCreateFollowRequest = () => HttpServerRequest.schemaBodyJson(CreateFollowRequestSchema) @@ -615,8 +624,74 @@ const resolveRequestOrigin = (request: HttpServerRequest.HttpServerRequest): str return `${proto}://${host}` } -const resolveSkillerBackendUrl = (request: HttpServerRequest.HttpServerRequest): string => - resolveDockerGitSkillerBackendUrl(process.env, resolveRequestOrigin(request)) +// CHANGE: Preserve the docker-git web `/api` proxy prefix in Skiller Web callbacks. +// WHY: External Skiller Web runs on another origin and must call the browser-reachable API URL. +// QUOTE(ТЗ): "у нас всё равно юзается http://192.168.0.206:4174/api/ssh/session/.../skiller/app/" +// REF: user-message-2026-06-18-skiller-external-module +// SOURCE: n/a +// FORMAT THEOREM: forwardedPrefix = p_safe -> backendUrl = origin + normalize(p_safe) +// PURITY: SHELL +// EFFECT: Reads trusted reverse-proxy request headers. +// INVARIANT: Unsafe or root prefixes do not change the request origin. +// COMPLEXITY: O(n) where n = |x-forwarded-prefix|. +const normalizeForwardedPrefix = (value: string | undefined): string => { + const prefix = firstCommaValue(value) + if (prefix === undefined || prefix.length === 0 || prefix === "/") { + return "" + } + if (!prefix.startsWith("/") || prefix.includes("://")) { + return "" + } + return prefix.replace(/\/+$/u, "") +} + +// CHANGE: Give hosted Skiller Web an HTTPS-reachable docker-git backend URL. +// WHY: Chrome blocks an HTTPS Vercel app from fetching a plain HTTP LAN API, even when CORS/PNA preflight succeeds. +// QUOTE(ТЗ): "сделать что бы оно начало работать" +// REF: user-message-2026-06-18-skiller-vercel-failed-fetch +// SOURCE: n/a +const dockerGitApiPort = (env: Record): string => { + const configured = env["DOCKER_GIT_API_PORT"]?.trim() + return configured !== undefined && /^\d{1,5}$/u.test(configured) ? configured : "3334" +} + +const firstControllerIpv4 = (): string => + Object.values(networkInterfaces()) + .flatMap((entries) => entries ?? []) + .find((entry) => entry.family === "IPv4" && !entry.internal)?.address ?? "127.0.0.1" + +const internalSkillerTunnelPanelUrl = ( + env: Record +): string => + `http://${firstControllerIpv4()}:${dockerGitApiPort(env)}` + +// FORMAT THEOREM: externalWebEnabled ∧ requestBackendUrl.protocol != https ∧ noConfiguredBackend -> backendUrl = cloudflare(internalApiUrl) + prefix +// PURITY: SHELL +// EFFECT: Reads controller network interfaces and may start or reuse the panel Cloudflare Quick Tunnel. +// INVARIANT: Explicit backend env values and already-HTTPS request URLs do not start a tunnel. +// COMPLEXITY: O(t) where t is the bounded tunnel startup wait. +const resolveSkillerBackendUrl = ( + request: HttpServerRequest.HttpServerRequest +): Effect.Effect => { + const requestOrigin = resolveRequestOrigin(request) + const forwardedPrefix = normalizeForwardedPrefix(readHeader(request, "x-forwarded-prefix")) + const decision = resolveDockerGitSkillerBackendUrlDecision(process.env, requestOrigin, forwardedPrefix) + return Match.value(decision).pipe( + Match.when({ _tag: "Configured" }, ({ backendUrl }) => Effect.succeed(backendUrl)), + Match.when({ _tag: "Request" }, ({ backendUrl }) => Effect.succeed(backendUrl)), + Match.when({ _tag: "Tunnel" }, ({ forwardedPrefix: tunnelPrefix }) => + startPanelCloudflareTunnel({ panelUrl: internalSkillerTunnelPanelUrl(process.env) }).pipe( + Effect.flatMap((tunnel) => + tunnel.publicUrl === null + ? Effect.fail(new ApiInternalError({ + message: "Cloudflare tunnel did not return a public URL for Skiller backend." + })) + : Effect.succeed(joinSkillerBackendUrl(tunnel.publicUrl, tunnelPrefix)) + ) + )), + Match.exhaustive + ) +} const isPrivateNetworkCorsRequest = ( request: HttpServerRequest.HttpServerRequest @@ -916,6 +991,7 @@ const skillerConnectInfoResponse = ( request: HttpServerRequest.HttpServerRequest ) => listProjects().pipe( + Effect.flatMap(inspectableSkillerConnectProjects), Effect.flatMap((projects) => skillerJsonResponse(request, { ok: true, projects }, 200)), Effect.catchAll((error) => skillerErrorResponse(request, error)) ) @@ -929,10 +1005,11 @@ const skillerConnectResponse = ( if (projectKey.length === 0) { return yield* _(Effect.fail(new ApiBadRequestError({ message: "projectKey is required." }))) } + const backendUrl = yield* _(resolveSkillerBackendUrl(request)) const connection = yield* _(connectSkillerWeb( projectKey, normalizedOptionalString(body.sessionId), - resolveSkillerBackendUrl(request) + backendUrl )) return yield* _(skillerJsonResponse(request, { ok: true, ...connection }, 202)) }).pipe( @@ -977,11 +1054,28 @@ export const makeRouter = () => { return yield* _(skillerConnectResponse(request)) }) ), + HttpRouter.get( + "/skiller/external-launch/:launchId", + Effect.gen(function*(_) { + const { launchId } = yield* _(skillerExternalLaunchParams) + const html = yield* _(readExternalSkillerLaunchHtml(launchId)) + return yield* _(textResponse(html, "text/html; charset=utf-8", 200)) + }).pipe(Effect.catchAll(errorResponse)) + ), + HttpRouter.get( + "/api/skiller/external-launch/:launchId", + Effect.gen(function*(_) { + const { launchId } = yield* _(skillerExternalLaunchParams) + const html = yield* _(readExternalSkillerLaunchHtml(launchId)) + return yield* _(textResponse(html, "text/html; charset=utf-8", 200)) + }).pipe(Effect.catchAll(errorResponse)) + ), HttpRouter.post( "/skiller/open", Effect.gen(function*(_) { const request = yield* _(HttpServerRequest.HttpServerRequest) - const launch = yield* _(openSkiller(undefined, undefined, resolveSkillerBackendUrl(request))) + const backendUrl = yield* _(resolveSkillerBackendUrl(request)) + const launch = yield* _(openSkiller(undefined, undefined, backendUrl)) return yield* _(jsonResponse({ ok: true, ...launch }, 202)) }).pipe(Effect.catchAll(errorResponse)) ), @@ -990,7 +1084,8 @@ export const makeRouter = () => { Effect.gen(function*(_) { const request = yield* _(HttpServerRequest.HttpServerRequest) const { projectKey } = yield* _(projectKeyParams) - const launch = yield* _(openSkiller(projectKey, undefined, resolveSkillerBackendUrl(request))) + const backendUrl = yield* _(resolveSkillerBackendUrl(request)) + const launch = yield* _(openSkiller(projectKey, undefined, backendUrl)) return yield* _(jsonResponse({ ok: true, ...launch }, 202)) }).pipe(Effect.catchAll(errorResponse)) ), @@ -999,7 +1094,8 @@ export const makeRouter = () => { Effect.gen(function*(_) { const request = yield* _(HttpServerRequest.HttpServerRequest) const { projectKey, sessionId } = yield* _(terminalSessionByProjectKeyParams) - const launch = yield* _(openSkillerForTerminalSession(projectKey, sessionId, resolveSkillerBackendUrl(request))) + const backendUrl = yield* _(resolveSkillerBackendUrl(request)) + const launch = yield* _(openSkillerForTerminalSession(projectKey, sessionId, backendUrl)) return yield* _(jsonResponse({ ok: true, ...launch }, 202)) }).pipe(Effect.catchAll(errorResponse)) ), diff --git a/packages/api/src/services/project-browser-core.ts b/packages/api/src/services/project-browser-core.ts index aefad337..32719515 100644 --- a/packages/api/src/services/project-browser-core.ts +++ b/packages/api/src/services/project-browser-core.ts @@ -25,7 +25,7 @@ export const renderProjectBrowserNoVncPath = (projectId: string): string => { const projectKey = projectShortKey(projectId) const params = new URLSearchParams({ autoconnect: "true", - resize: "remote", + resize: "scale", path: `b/${projectKey}/websockify` }) return `/b/${projectKey}/vnc.html?${params.toString()}` diff --git a/packages/api/src/services/project-browser.ts b/packages/api/src/services/project-browser.ts index 0f12ab4d..1a741a30 100644 --- a/packages/api/src/services/project-browser.ts +++ b/packages/api/src/services/project-browser.ts @@ -275,6 +275,98 @@ const waitForBrowserRuntimeReady = ( ) ) +const renderBrowserWindowManagerRepairScript = (): string => [ + "set -eu", + "changed=0", + "chromium_launcher=/usr/local/bin/docker-git-chromium-launch", + "tmp_launcher=\"$(mktemp)\"", + "cat > \"$tmp_launcher\" <<'DOCKER_GIT_CHROMIUM_LAUNCH'", + "#!/bin/sh", + "set -eu", + "rm -rf /data/Default/Sessions", + "if [ -f /data/Default/Preferences ]; then", + " tmp_preferences=\"$(mktemp)\"", + " sed -e 's#\"exit_type\":\"Crashed\"#\"exit_type\":\"Normal\"#g' \\", + " -e 's#\"exited_cleanly\":false#\"exited_cleanly\":true#g' \\", + " /data/Default/Preferences > \"$tmp_preferences\" \\", + " && cat \"$tmp_preferences\" > /data/Default/Preferences", + " rm -f \"$tmp_preferences\"", + "fi", + "exec /usr/bin/chromium-browser \\", + " --no-sandbox \\", + " --disable-dev-shm-usage \\", + " --disable-gpu \\", + " --disable-background-networking \\", + " --disable-component-update \\", + " --disable-default-apps \\", + " --disable-extensions \\", + " --disable-sync \\", + " --no-first-run \\", + " --no-default-browser-check \\", + " --disable-session-crashed-bubble \\", + " --hide-crash-restore-bubble \\", + " --start-maximized \\", + " --remote-debugging-port=9222 \\", + " --user-data-dir=/data \\", + " --display=:99 \\", + " about:blank", + "DOCKER_GIT_CHROMIUM_LAUNCH", + "if [ ! -f \"$chromium_launcher\" ] || ! cmp -s \"$tmp_launcher\" \"$chromium_launcher\"; then", + " mv \"$tmp_launcher\" \"$chromium_launcher\"", + " chmod +x \"$chromium_launcher\"", + " changed=1", + "else", + " rm -f \"$tmp_launcher\"", + "fi", + "for supervisor_file in /etc/supervisor.d/*.ini /etc/supervisor/conf.d/*.conf; do", + " [ -f \"$supervisor_file\" ] || continue", + " before=\"$(cat \"$supervisor_file\")\"", + " sed -i -E \\", + " -e 's#Xvfb :99 -screen 0 1024x768x16#Xvfb :99 -screen 0 1024x768x24#g' \\", + " -e 's#^command=/usr/bin/fluxbox$#command=/usr/bin/env DISPLAY=:99 /usr/bin/fluxbox#g' \\", + " -e 's#^command=/usr/bin/chromium-browser .*$#command=/usr/local/bin/docker-git-chromium-launch#g' \\", + " \"$supervisor_file\"", + " after=\"$(cat \"$supervisor_file\")\"", + " if [ \"$before\" != \"$after\" ]; then changed=1; fi", + "done", + "if [ \"$changed\" = \"1\" ]; then echo changed; exit 0; fi", + "if ! pgrep -x fluxbox >/dev/null 2>&1; then echo restart; exit 0; fi", + "echo unchanged" +].join("\n") + +const restartBrowserContainer = ( + cwd: string, + containerName: string +) => + dockerCapture( + cwd, + ["restart", containerName], + "docker restart repaired browser" + ).pipe(Effect.asVoid) + +const repairBrowserWindowManager = ( + cwd: string, + containerName: string +) => + dockerCapture( + cwd, + ["exec", containerName, "bash", "-lc", renderBrowserWindowManagerRepairScript()], + "docker exec browser window manager repair" + ).pipe( + Effect.flatMap((output) => + output.includes("changed") || output.includes("restart") + ? restartBrowserContainer(cwd, containerName) + : Effect.void + ), + Effect.mapError((error) => + new ApiConflictError({ + message: `Failed to prepare browser window manager for ${containerName}: ${ + error instanceof Error ? error.message : String(error) + }` + }) + ) + ) + const parseContainerNetworkEntries = (output: string): ReadonlyArray => output .trim() @@ -466,6 +558,7 @@ export const startProjectBrowserSession = ( const project = yield* _(getProjectItemById(projectId)) const containerName = browserContainerName(project.containerName) yield* _(startBrowserContainer(project.projectDir, project.containerName)) + yield* _(repairBrowserWindowManager(project.projectDir, containerName)) yield* _(waitForBrowserRuntimeReady(project.projectDir, project.containerName)) const state = yield* _(inspectBrowserContainerState(project.projectDir, containerName)) return browserSessionFromState(projectId, containerName, state, externalOrigin) diff --git a/packages/api/src/services/skiller-core.ts b/packages/api/src/services/skiller-core.ts index 51217fe5..7951ca03 100644 --- a/packages/api/src/services/skiller-core.ts +++ b/packages/api/src/services/skiller-core.ts @@ -47,9 +47,41 @@ export type ExternalSkillerLaunchUrlInput = { readonly skillerWebUrl: string } +export type ExternalSkillerLaunchWrapperInput = { + readonly targetUrl: string +} + +export type DockerGitSkillerBackendUrlDecision = + | { readonly _tag: "Configured"; readonly backendUrl: string } + | { readonly _tag: "Request"; readonly backendUrl: string } + | { readonly _tag: "Tunnel"; readonly forwardedPrefix: string; readonly panelUrl: string } + +export type SkillerConnectProject = { + readonly status: "running" | "stopped" | "unknown" +} + const trimTrailingSlashes = (value: string): string => value.replace(/\/+$/u, "") +const firstNonEmptySkillerBackendUrl = ( + env: Record +): string | undefined => + [ + env["DOCKER_GIT_SKILLER_BACKEND_URL"], + env["DOCKER_GIT_API_PUBLIC_URL"] + ] + .map((value) => value?.trim()) + .find((value) => value !== undefined && value.length > 0) + +const isHttpsUrl = (value: string): boolean => + URL.canParse(value) && new URL(value).protocol === "https:" + +export const joinSkillerBackendUrl = ( + baseUrl: string, + forwardedPrefix: string +): string => + `${trimTrailingSlashes(baseUrl)}${forwardedPrefix}` + const configuredOrigin = (raw: string): string | null => { const value = raw.trim() if (!URL.canParse(value)) { @@ -94,15 +126,46 @@ export const resolveDockerGitSkillerBackendUrl = ( env: Record, requestOrigin: string ): string => { - const configured = [ - env["DOCKER_GIT_SKILLER_BACKEND_URL"], - env["DOCKER_GIT_API_PUBLIC_URL"] - ] - .map((value) => value?.trim()) - .find((value) => value !== undefined && value.length > 0) + const configured = firstNonEmptySkillerBackendUrl(env) return configured ?? requestOrigin } +/** + * Resolves how the docker-git API URL should be exposed to external Skiller Web. + * + * @param env - Environment map containing optional Skiller web/backend overrides. + * @param requestOrigin - Browser-visible request origin from trusted forwarding headers. + * @param forwardedPrefix - Normalized API proxy prefix, such as `/api` or an empty string. + * @returns A pure strategy: use configured URL, use HTTPS request URL, or start a panel tunnel. + * + * @pure true + * @effect none + * @invariant If an explicit backend URL exists, it is returned before derived URLs. + * @precondition forwardedPrefix is empty or starts with `/` and has no trailing slash. + * @postcondition Tunnel is selected only for enabled external Skiller Web over non-HTTPS request URLs. + * @complexity O(n) where n is the total configured URL length. + * @throws Never + */ +export const resolveDockerGitSkillerBackendUrlDecision = ( + env: Record, + requestOrigin: string, + forwardedPrefix: string +): DockerGitSkillerBackendUrlDecision => { + const configured = firstNonEmptySkillerBackendUrl(env) + if (configured !== undefined) { + return { _tag: "Configured", backendUrl: configured } + } + + const requestBackendUrl = joinSkillerBackendUrl(requestOrigin, forwardedPrefix) + if (isHttpsUrl(requestBackendUrl)) { + return { _tag: "Request", backendUrl: requestBackendUrl } + } + + return resolveConfiguredSkillerWebUrl(env)._tag === "Enabled" + ? { _tag: "Tunnel", forwardedPrefix, panelUrl: requestOrigin } + : { _tag: "Request", backendUrl: requestBackendUrl } +} + export const configuredSkillerWebCorsOrigins = ( env: Record ): ReadonlyArray => { @@ -127,6 +190,25 @@ export const isSkillerWebCorsOriginAllowed = ( ): boolean => origin !== undefined && configuredSkillerWebCorsOrigins(env).includes(origin) +/** + * Keeps the Skiller external project picker scoped to currently usable projects. + * + * @param projects - Docker-git project summaries with conservative runtime status. + * @returns Projects whose known status is `running`. + * + * @pure true + * @effect none + * @invariant every returned project has status = running. + * @precondition status belongs to the ProjectStatus finite domain. + * @postcondition stopped and unknown projects are not visible in Skiller connect. + * @complexity O(n) time where n is project count; O(n) space for the filtered result. + * @throws Never + */ +export const activeSkillerConnectProjects = ( + projects: ReadonlyArray +): ReadonlyArray => + projects.filter((project) => project.status === "running") + export const externalSkillerLaunchUrl = (input: ExternalSkillerLaunchUrlInput): string => { const url = new URL(`${trimTrailingSlashes(input.skillerWebUrl)}/launch`) url.searchParams.set("backendUrl", input.backendUrl) @@ -139,6 +221,54 @@ export const externalSkillerLaunchUrl = (input: ExternalSkillerLaunchUrlInput): return url.toString() } +export const externalSkillerLaunchWrapperPath = (launchId: string): string => + `/api/skiller/external-launch/${encodeURIComponent(launchId)}` + +const htmlAttributeReplacements: Readonly> = { + "\"": """, + "&": "&", + "<": "<", + ">": ">" +} + +export const escapeHtmlAttribute = (value: string): string => + value.replace(/[&"<>]/gu, (character) => htmlAttributeReplacements[character] ?? character) + +export const externalSkillerLaunchWrapperHtml = (input: ExternalSkillerLaunchWrapperInput): string => { + const targetUrl = escapeHtmlAttribute(input.targetUrl) + return ` + + + + + Skiller Web + + + + + + +` +} + export const parseDockerMountLines = (output: string): ReadonlyArray => output .split(/\r?\n/u) diff --git a/packages/api/src/services/skiller.ts b/packages/api/src/services/skiller.ts index 6bc820e6..da667603 100644 --- a/packages/api/src/services/skiller.ts +++ b/packages/api/src/services/skiller.ts @@ -1,4 +1,5 @@ import { spawn, type ChildProcess, type SpawnOptions } from "node:child_process" +import { randomUUID } from "node:crypto" import { chmodSync, chownSync, closeSync, existsSync, mkdirSync, openSync, readFileSync, statSync } from "node:fs" import { createServer } from "node:net" import { homedir } from "node:os" @@ -6,7 +7,6 @@ import { dirname, join, resolve } from "node:path" import { recordProjectRuntimeActivity } from "@effect-template/lib" import { runCommandCapture } from "@effect-template/lib/shell/command-runner" import { CommandFailedError } from "@effect-template/lib/shell/errors" -import type { ProjectItem } from "@effect-template/lib/usecases/projects" import type { ListProjectsContext } from "@effect-template/lib/usecases/projects-list" import { NodeContext } from "@effect/platform-node" import type { PlatformError } from "@effect/platform/Error" @@ -18,7 +18,10 @@ import * as Stream from "effect/Stream" import { ApiConflictError, ApiInternalError, ApiNotFoundError } from "../api/errors.js" import { + activeSkillerConnectProjects, containerCodexSkillsPath, + externalSkillerLaunchWrapperHtml, + externalSkillerLaunchWrapperPath, externalSkillerLaunchUrl, parseDockerMountLines, remapContainerPathToMountedHost, @@ -101,14 +104,39 @@ type SkillerBrowserScopeSelection = { readonly sessionId: string | null } +type ExternalSkillerLaunchRecord = { + readonly expiresAtEpochMs: number + readonly targetUrl: string +} + +type SkillerConnectProject = { + readonly containerName: string + readonly envGlobalPath: string + readonly projectDir: string + readonly projectKey: string + readonly sshUser: string + readonly status: "running" | "stopped" | "unknown" + readonly targetDir: string +} + +type SkillerScopeProject = { + readonly containerName: string + readonly envGlobalPath: string + readonly projectDir: string + readonly sshUser: string + readonly targetDir: string +} + const submoduleRelativePath = join("third_party", "skiller-desktop-skills-manager") const launchLogPath = join(homedir(), ".docker-git", "logs", "skiller.log") const skillerAppPath = "/api/skiller/app/" const skillerTrpcBasePath = "/api/skiller" const skillerPreferredTrpcPort = 17888 +const externalSkillerLaunchTtlMs = 60 * 60 * 1_000 let currentProcess: SkillerProcess | null = null const sessionScopes = new Map() +const externalSkillerLaunches = new Map() const isRunning = (process: ChildProcess): boolean => process.exitCode === null && process.signalCode === null && !process.killed @@ -313,7 +341,7 @@ const containerHomePath = (sshUser: string): string => `/home/${sshUser}` const inspectContainerMounts = ( containerName: string -): Effect.Effect, ApiInternalError> => +): Effect.Effect, ApiConflictError | ApiInternalError> => dockerCapture( [ "inspect", @@ -322,7 +350,14 @@ const inspectContainerMounts = ( containerName ], "docker inspect mounts" - ).pipe(Effect.map(parseDockerMountLines)) + ).pipe( + Effect.mapError(() => + new ApiConflictError({ + message: `Skiller cannot inspect Docker container ${containerName}. Start or recreate the project before connecting Skiller.` + }) + ), + Effect.map(parseDockerMountLines) + ) const requireAccessibleDirectory = ( path: string, @@ -341,7 +376,7 @@ const requireAccessibleDirectory = ( const resolveSkillerScope = ( projectKey: string, - project: ProjectItem + project: SkillerScopeProject ): Effect.Effect => Effect.gen(function*(_) { const mounts = yield* _(inspectContainerMounts(project.containerName)) @@ -395,6 +430,37 @@ const resolveRequestedSkillerScope = ( Effect.flatMap((project) => resolveSkillerScope(projectKey, project)) ) +/** + * Filters the external Skiller project picker to projects that can actually be mounted. + * + * @param projects - Docker-git API project summaries. + * @returns Effect containing only projects whose current Docker container can be inspected and mapped. + * + * @pure false + * @effect Runs `docker inspect` and filesystem access checks for candidate running projects. + * @invariant every returned project has status = running and resolveSkillerScope succeeds at filter time. + * @precondition projects are decoded API DTOs with stable project keys and container names. + * @postcondition stale `last known: running` projects with missing containers are not offered to Skiller Web. + * @complexity O(n * docker inspect) time for n candidate projects; O(n) space. + * @throws Never - inaccessible candidates are represented by absence from the returned array. + */ +export const inspectableSkillerConnectProjects = ( + projects: ReadonlyArray +): Effect.Effect, never> => + Effect.forEach( + activeSkillerConnectProjects(projects), + (project) => + resolveSkillerScope(project.projectKey, project).pipe( + Effect.as(project), + Effect.catchAll(() => Effect.succeed(null)) + ), + { concurrency: 8 } + ).pipe( + Effect.map((projectsOrNulls) => + projectsOrNulls.filter((project): project is Project => project !== null) + ) + ) + export const readSkillerProjectContext = ( projectKey: string, sessionId: string | null @@ -447,7 +513,7 @@ const prepareLaunchScript = [ "DOCKER_GIT_SKILLER_PATCH_MARKER=out/.docker-git-browser-folder-picker.patch", "if [ -f ../../scripts/skiller-apply-docker-git-patches.mjs ]; then bun ../../scripts/skiller-apply-docker-git-patches.mjs; fi", "if [ ! -d node_modules ]; then bun install --frozen-lockfile; fi", - "if [ ! -f out/main/index.js ] || [ ! -f out/renderer/index.html ] || { [ -f \"$DOCKER_GIT_SKILLER_PATCH\" ] && [ ! -f \"$DOCKER_GIT_SKILLER_PATCH_MARKER\" ]; } || { [ -f \"$DOCKER_GIT_SKILLER_PATCH\" ] && [ \"$DOCKER_GIT_SKILLER_PATCH\" -nt \"$DOCKER_GIT_SKILLER_PATCH_MARKER\" ]; }; then", + "if [ ! -f out/main/index.js ] || [ ! -f out/renderer/index.html ] || { [ -f \"$DOCKER_GIT_SKILLER_PATCH\" ] && [ ! -f \"$DOCKER_GIT_SKILLER_PATCH_MARKER\" ]; }; then", " bun run build", " mkdir -p out", " touch \"$DOCKER_GIT_SKILLER_PATCH_MARKER\"", @@ -825,6 +891,46 @@ const rememberSessionScope = (sessionId: string | undefined, scope: SkillerConta } } +const pruneExternalSkillerLaunches = (nowEpochMs: number): void => { + for (const [launchId, record] of externalSkillerLaunches) { + if (record.expiresAtEpochMs <= nowEpochMs) { + externalSkillerLaunches.delete(launchId) + } + } +} + +const registerExternalSkillerLaunch = (targetUrl: string, nowEpochMs: number): string => { + pruneExternalSkillerLaunches(nowEpochMs) + const launchId = randomUUID() + externalSkillerLaunches.set(launchId, { + expiresAtEpochMs: nowEpochMs + externalSkillerLaunchTtlMs, + targetUrl + }) + return externalSkillerLaunchWrapperPath(launchId) +} + +export const readExternalSkillerLaunchTarget = ( + launchId: string +): Effect.Effect => + Effect.sync(() => { + const nowEpochMs = Date.now() + pruneExternalSkillerLaunches(nowEpochMs) + return externalSkillerLaunches.get(launchId)?.targetUrl ?? null + }).pipe( + Effect.flatMap((targetUrl) => + targetUrl === null + ? Effect.fail(new ApiNotFoundError({ message: "Skiller launch link expired or was not found." })) + : Effect.succeed(targetUrl) + ) + ) + +export const readExternalSkillerLaunchHtml = ( + launchId: string +): Effect.Effect => + readExternalSkillerLaunchTarget(launchId).pipe( + Effect.map((targetUrl) => externalSkillerLaunchWrapperHtml({ targetUrl })) + ) + const touchSkillerActivity = ( scope: SkillerContainerScope | null ): Effect.Effect => @@ -854,23 +960,25 @@ const externalSkillerLaunch = ( if (config._tag === "Invalid") { return Effect.fail(new ApiInternalError({ message: config.message })) } - const appPath = externalSkillerLaunchUrl({ - backendUrl, - projectKey, - sessionId, - skillerWebUrl: config.baseUrl - }) - return Effect.succeed({ - alreadyRunning: true, - appPath, - backendUrl, - logPath: "", - mode: "external", - pid: null, - scope, - startedAtIso: new Date().toISOString(), - trpcBasePath: `${config.baseUrl}/trpc`, - trpcPort: 0 + return Effect.sync(() => { + const targetUrl = externalSkillerLaunchUrl({ + backendUrl, + projectKey, + sessionId, + skillerWebUrl: config.baseUrl + }) + return { + alreadyRunning: true, + appPath: registerExternalSkillerLaunch(targetUrl, Date.now()), + backendUrl, + logPath: "", + mode: "external", + pid: null, + scope, + startedAtIso: new Date().toISOString(), + trpcBasePath: `${config.baseUrl}/trpc`, + trpcPort: 0 + } }) } diff --git a/packages/api/src/services/terminal-sessions.ts b/packages/api/src/services/terminal-sessions.ts index e740d9e3..13a1d091 100644 --- a/packages/api/src/services/terminal-sessions.ts +++ b/packages/api/src/services/terminal-sessions.ts @@ -1782,6 +1782,29 @@ const denyUpgrade = (socket: Duplex): void => { socket.destroy() } +export type TerminalWebSocketUpgradeServer = { + readonly handleUpgrade: ( + request: IncomingMessage, + socket: Duplex, + head: Buffer, + callback: (webSocket: WebSocket) => void + ) => void +} + +export const safeHandleTerminalWebSocketUpgrade = ( + webSocketServer: TerminalWebSocketUpgradeServer, + request: IncomingMessage, + socket: Duplex, + head: Buffer, + onWebSocket: (webSocket: WebSocket) => void +): void => { + try { + webSocketServer.handleUpgrade(request, socket, head, onWebSocket) + } catch { + denyUpgrade(socket) + } +} + const resolveParsedTerminalRecord = ( parsed: ParsedTerminalPath ): Effect.Effect => @@ -1804,7 +1827,7 @@ export const attachTerminalWebSocketServer = (server: HttpServer): void => { denyUpgrade(socket) }, onSuccess: (record) => { - webSocketServer.handleUpgrade(request, socket, head, (webSocket: WebSocket) => { + safeHandleTerminalWebSocketUpgrade(webSocketServer, request, socket, head, (webSocket: WebSocket) => { try { attachSocketToRecord(record, webSocket, parsed.cols, parsed.rows) } catch (error) { diff --git a/packages/api/tests/project-browser.test.ts b/packages/api/tests/project-browser.test.ts index d9c8a814..186cfe3f 100644 --- a/packages/api/tests/project-browser.test.ts +++ b/packages/api/tests/project-browser.test.ts @@ -90,6 +90,21 @@ describe("project browser", () => { [0], expect.any(Function) ) + expect(runCommandCaptureMock).toHaveBeenCalledWith( + { + args: [ + "exec", + browserContainerName, + "bash", + "-lc", + expect.stringMatching(/docker-git-chromium-launch[\s\S]*--hide-crash-restore-bubble[\s\S]*--display=:99/u) + ], + command: "docker", + cwd: projectDir + }, + [0], + expect.any(Function) + ) expect(runCommandCaptureMock).toHaveBeenLastCalledWith( { args: ["inspect", "-f", "{{.Id}}\t{{.State.Running}}\t{{.State.Status}}", browserContainerName], @@ -115,4 +130,30 @@ describe("project browser", () => { expect(result.left.message).toContain("Playwright MCP is enabled") } }).pipe(Effect.provide(NodeContext.layer))) + + it.effect("restarts the browser container after repairing the sidecar supervisor config", () => + Effect.gen(function*(_) { + runCommandCaptureMock.mockImplementation((command: { readonly args: ReadonlyArray }) => { + if (command.args[0] === "inspect") { + return Effect.succeed("browser-container-id\ttrue\trunning") + } + if (command.args[0] === "exec" && command.args[1] === browserContainerName) { + return Effect.succeed("changed\n") + } + return Effect.succeed("Browser started") + }) + + const browser = yield* _(startProjectBrowserSession(projectId, "http://127.0.0.1:3334")) + + expect(browser.status).toBe("running") + expect(runCommandCaptureMock).toHaveBeenCalledWith( + { + args: ["restart", browserContainerName], + command: "docker", + cwd: projectDir + }, + [0], + expect.any(Function) + ) + }).pipe(Effect.provide(NodeContext.layer))) }) diff --git a/packages/api/tests/project-port-forward-core.test.ts b/packages/api/tests/project-port-forward-core.test.ts index cf71cd72..cf64154c 100644 --- a/packages/api/tests/project-port-forward-core.test.ts +++ b/packages/api/tests/project-port-forward-core.test.ts @@ -144,6 +144,7 @@ describe("project port forward core", () => { expect(renderProjectBrowserProxyPath("a/b")).toBe(`/b/${key}/`) expect(renderProjectBrowserCdpPath("a/b")).toBe(`/b/${key}/cdp/json/version`) expect(renderProjectBrowserNoVncPath("a/b")).toContain(`/b/${key}/vnc.html?`) + expect(renderProjectBrowserNoVncPath("a/b")).toContain("resize=scale") expect(renderProjectBrowserNoVncPath("a/b")).toContain(`path=b%2F${key}%2Fwebsockify`) })) diff --git a/packages/api/tests/skiller-core.test.ts b/packages/api/tests/skiller-core.test.ts index a01572c2..8b7f322a 100644 --- a/packages/api/tests/skiller-core.test.ts +++ b/packages/api/tests/skiller-core.test.ts @@ -1,16 +1,22 @@ import { describe, expect, it } from "@effect/vitest" import { + activeSkillerConnectProjects, configuredSkillerWebCorsOrigins, containerCodexSkillsPath, + escapeHtmlAttribute, + externalSkillerLaunchWrapperHtml, + externalSkillerLaunchWrapperPath, externalSkillerLaunchUrl, isSkillerWebCorsOriginAllowed, + joinSkillerBackendUrl, parseDockerMountLines, remapContainerPathToMountedHost, remapSkillerBrowserContainerPath, remapSkillerBrowserHostPath, resolveConfiguredSkillerWebUrl, resolveDockerGitSkillerBackendUrl, + resolveDockerGitSkillerBackendUrlDecision, sameSkillerScope, skillerBrowserScopeForContainer } from "../src/services/skiller-core.js" @@ -43,6 +49,30 @@ describe("skiller container filesystem mapping", () => { expect(launchUrl.searchParams.get("sessionId")).toBe("session/1") }) + it("builds short external Skiller wrapper paths and escaped HTML", () => { + expect(externalSkillerLaunchWrapperPath("launch/id one")).toBe( + "/api/skiller/external-launch/launch%2Fid%20one" + ) + expect(escapeHtmlAttribute("https://example.test/?a=1&b=\"\"")).toBe( + "https://example.test/?a=1&b="<x>"" + ) + + const html = externalSkillerLaunchWrapperHtml({ + targetUrl: "https://skiller.example/launch?backendUrl=https://api.example/a&projectKey=p1" + }) + + expect(html).toContain(" { expect(resolveDockerGitSkillerBackendUrl({ DOCKER_GIT_API_PUBLIC_URL: "https://public-api.example", @@ -56,6 +86,45 @@ describe("skiller container filesystem mapping", () => { expect(resolveDockerGitSkillerBackendUrl({}, "http://localhost:3334")).toBe("http://localhost:3334") }) + it("selects the Skiller backend exposure strategy without shell effects", () => { + expect(resolveDockerGitSkillerBackendUrlDecision({ + DOCKER_GIT_SKILLER_WEB_URL: "https://skiller-web-henna.vercel.app", + DOCKER_GIT_SKILLER_BACKEND_URL: "https://manual-backend.example/api" + }, "http://192.168.0.206:4174", "/api")).toEqual({ + _tag: "Configured", + backendUrl: "https://manual-backend.example/api" + }) + + expect(resolveDockerGitSkillerBackendUrlDecision({ + DOCKER_GIT_SKILLER_WEB_URL: "https://skiller-web-henna.vercel.app" + }, "https://public-panel.example", "/api")).toEqual({ + _tag: "Request", + backendUrl: "https://public-panel.example/api" + }) + + expect(resolveDockerGitSkillerBackendUrlDecision({ + DOCKER_GIT_SKILLER_WEB_URL: "https://skiller-web-henna.vercel.app" + }, "http://192.168.0.206:4174", "/api")).toEqual({ + _tag: "Tunnel", + forwardedPrefix: "/api", + panelUrl: "http://192.168.0.206:4174" + }) + + expect(resolveDockerGitSkillerBackendUrlDecision({}, "http://192.168.0.206:4174", "/api")).toEqual({ + _tag: "Request", + backendUrl: "http://192.168.0.206:4174/api" + }) + }) + + it("joins public tunnel URLs with the API forwarding prefix", () => { + expect(joinSkillerBackendUrl("https://panel.trycloudflare.com/", "/api")).toBe( + "https://panel.trycloudflare.com/api" + ) + expect(joinSkillerBackendUrl("https://panel.trycloudflare.com", "")).toBe( + "https://panel.trycloudflare.com" + ) + }) + it("allows Skiller Web CORS only from configured exact origins and local dev", () => { const env = { DOCKER_GIT_SKILLER_ALLOWED_ORIGINS: "https://preview.example/app, file:///tmp/ignored", @@ -76,6 +145,18 @@ describe("skiller container filesystem mapping", () => { expect(isSkillerWebCorsOriginAllowed(undefined, env)).toBe(false) }) + it("keeps only active projects in the external Skiller connect picker", () => { + const projects = [ + { projectKey: "running-project", status: "running" }, + { projectKey: "stopped-project", status: "stopped" }, + { projectKey: "unknown-project", status: "unknown" } + ] as const + + expect(activeSkillerConnectProjects(projects)).toEqual([ + { projectKey: "running-project", status: "running" } + ]) + }) + it("maps a project container path through the most specific writable Docker mount", () => { const mounts = parseDockerMountLines([ "/var/lib/docker/volumes/project-home/_data\t/home/dev\ttrue", diff --git a/packages/api/tests/skiller-cors.test.ts b/packages/api/tests/skiller-cors.test.ts index abce77b6..8f6c7ab8 100644 --- a/packages/api/tests/skiller-cors.test.ts +++ b/packages/api/tests/skiller-cors.test.ts @@ -33,6 +33,89 @@ const skillerPrivateNetworkPreflight = (path: string) => } }) +const setOptionalEnv = (key: string, value: string | undefined): void => { + if (value === undefined) { + delete process.env[key] + return + } + process.env[key] = value +} + +const withTemporaryEnv = ( + entries: ReadonlyArray, + effect: Effect.Effect +): Effect.Effect => + Effect.acquireUseRelease( + Effect.sync(() => { + const previousEntries: Array = [] + for (const [key, value] of entries) { + previousEntries.push([key, process.env[key]]) + setOptionalEnv(key, value) + } + return previousEntries + }), + () => effect, + (previousEntries) => + Effect.sync(() => { + for (const [key, value] of previousEntries) { + setOptionalEnv(key, value) + } + }) + ) + +const readStringProperty = (value: object, key: string): string | null => { + const field = Reflect.get(value, key) + return typeof field === "string" ? field : null +} + +const readLaunchResponse = (response: Response) => + Effect.tryPromise({ + try: () => response.text(), + catch: (cause) => new Error(String(cause)) + }).pipe( + Effect.flatMap((text) => + Effect.try({ + try: (): unknown => JSON.parse(text), + catch: (cause) => new Error(String(cause)) + }) + ), + Effect.flatMap((value) => { + if (typeof value !== "object" || value === null) { + return Effect.fail(new Error("Expected Skiller launch response object.")) + } + const appPath = readStringProperty(value, "appPath") + const backendUrl = readStringProperty(value, "backendUrl") + return appPath === null || backendUrl === null + ? Effect.fail(new Error("Expected Skiller launch appPath and backendUrl strings.")) + : Effect.succeed({ appPath, backendUrl }) + }) + ) + +const readTextResponse = (response: Response) => + Effect.tryPromise({ + try: () => response.text(), + catch: (cause) => new Error(String(cause)) + }) + +const requestExternalLaunch = ( + headers: Record, + env: ReadonlyArray = [] +) => + withTemporaryEnv( + [ + ["DOCKER_GIT_SKILLER_WEB_URL", SKILLER_WEB_PRODUCTION_ORIGIN], + ...env + ], + Effect.gen(function*(_) { + const response = yield* _(requestApiRoute("/skiller/open", { + method: "POST", + headers + })) + const body = yield* _(readLaunchResponse(response)) + return { body, response } + }) + ) + describe("skiller web CORS", () => { it("allows production Skiller Web private-network preflight requests", () => Effect.runPromise( @@ -57,6 +140,82 @@ describe("skiller web CORS", () => { }) )) + it("builds external launch URLs with the forwarded web proxy prefix", () => + Effect.runPromise( + Effect.gen(function*(_) { + const { body, response } = yield* _(requestExternalLaunch({ + "x-forwarded-host": "192.168.0.206:4174", + "x-forwarded-prefix": "/api", + "x-forwarded-proto": "https" + })) + const wrapperResponse = yield* _(requestApiRoute(body.appPath, { + method: "GET", + headers: {} + })) + const wrapperHtml = yield* _(readTextResponse(wrapperResponse)) + + expect(response.status).toBe(202) + expect(body.backendUrl).toBe("https://192.168.0.206:4174/api") + expect(body.appPath).toMatch(/^\/api\/skiller\/external-launch\/[0-9a-f-]+$/u) + expect(body.appPath).not.toContain("backendUrl") + expect(wrapperResponse.status).toBe(200) + expect(wrapperResponse.headers.get("content-type")).toContain("text/html") + expect(wrapperHtml).toContain(`${SKILLER_WEB_PRODUCTION_ORIGIN}/launch?`) + expect(wrapperHtml).toContain("backendUrl=https%3A%2F%2F192.168.0.206%3A4174%2Fapi") + }) + )) + + it("normalizes safe forwarded prefixes and ignores unsafe ones for external launch URLs", () => + Effect.runPromise( + Effect.gen(function*(_) { + const baseHeaders = { + "x-forwarded-host": "192.168.0.206:4174", + "x-forwarded-proto": "https" + } + const trailing = yield* _(requestExternalLaunch({ + ...baseHeaders, + "x-forwarded-prefix": "/api/" + })) + const root = yield* _(requestExternalLaunch({ + ...baseHeaders, + "x-forwarded-prefix": "/" + })) + const invalid = yield* _(requestExternalLaunch({ + ...baseHeaders, + "x-forwarded-prefix": "https://evil.example/api" + })) + + expect(trailing.body.backendUrl).toBe("https://192.168.0.206:4174/api") + expect(root.body.backendUrl).toBe("https://192.168.0.206:4174") + expect(invalid.body.backendUrl).toBe("https://192.168.0.206:4174") + }) + )) + + it("prefers explicit Skiller backend URL over forwarded request metadata", () => + Effect.runPromise( + Effect.gen(function*(_) { + const { body } = yield* _(requestExternalLaunch( + { + "x-forwarded-host": "192.168.0.206:4174", + "x-forwarded-prefix": "/api", + "x-forwarded-proto": "http" + }, + [["DOCKER_GIT_SKILLER_BACKEND_URL", "https://skiller-backend.example/custom"]] + )) + const wrapperResponse = yield* _(requestApiRoute(body.appPath, { + method: "GET", + headers: {} + })) + const wrapperHtml = yield* _(readTextResponse(wrapperResponse)) + + expect(body.backendUrl).toBe("https://skiller-backend.example/custom") + expect(body.appPath).toMatch(/^\/api\/skiller\/external-launch\/[0-9a-f-]+$/u) + expect(body.appPath).not.toContain("skiller-backend.example") + expect(wrapperResponse.status).toBe(200) + expect(wrapperHtml).toContain("backendUrl=https%3A%2F%2Fskiller-backend.example%2Fcustom") + }) + )) + it("rejects private-network preflight requests from unknown origins", () => Effect.runPromise( Effect.gen(function*(_) { diff --git a/packages/api/tests/skiller-routes.test.ts b/packages/api/tests/skiller-routes.test.ts index feb880ed..13e0074f 100644 --- a/packages/api/tests/skiller-routes.test.ts +++ b/packages/api/tests/skiller-routes.test.ts @@ -5,6 +5,7 @@ import { Effect } from "effect" import { openSkiller, parseSkillerRoute, + readExternalSkillerLaunchTarget, resolveSkillerBrowserScopeSelection, resolveSkillerRouteScopeSelection, runProcess, @@ -86,10 +87,16 @@ describe("skiller routes", () => { const launch = await Effect.runPromise( openSkiller(undefined, undefined, "https://docker-git.example").pipe(Effect.provide(NodeContext.layer)) ) - const launchUrl = new URL(launch.appPath) + const launchId = launch.appPath.split("/").at(-1) + if (launchId === undefined) { + throw new Error("Expected external Skiller launch id.") + } + const targetUrl = await Effect.runPromise(readExternalSkillerLaunchTarget(decodeURIComponent(launchId))) + const launchUrl = new URL(targetUrl) expect(launch.mode).toBe("external") expect(launch.alreadyRunning).toBe(true) + expect(launch.appPath).toMatch(/^\/api\/skiller\/external-launch\/[0-9a-f-]+$/u) expect(launch.backendUrl).toBe("https://docker-git.example") expect(launch.pid).toBeNull() expect(launch.trpcPort).toBe(0) diff --git a/packages/api/tests/terminal-sessions.test.ts b/packages/api/tests/terminal-sessions.test.ts index 2c79d969..32b19d8a 100644 --- a/packages/api/tests/terminal-sessions.test.ts +++ b/packages/api/tests/terminal-sessions.test.ts @@ -1,7 +1,11 @@ import { Effect } from "effect" +import { Buffer } from "node:buffer" import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs" +import { IncomingMessage } from "node:http" +import { Socket } from "node:net" import path from "node:path" import os from "node:os" +import { Duplex } from "node:stream" import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" import type { ProjectItem } from "@effect-template/lib" @@ -20,9 +24,11 @@ import { readProjectTerminalSessions, readProjectTerminalImage, renderTmuxAttachCommand, + safeHandleTerminalWebSocketUpgrade, setProjectActiveTerminalSession, startTerminalSession } from "../src/services/terminal-sessions.js" +import type { TerminalWebSocketUpgradeServer } from "../src/services/terminal-sessions.js" const listProjectItemsMock = vi.hoisted(() => vi.fn()) const prepareProjectSshMock = vi.hoisted(() => vi.fn()) @@ -172,6 +178,21 @@ const readPersistedActiveSessionId = (): string | null => { return typeof value === "string" ? value : null } +class MemoryDuplexSocket extends Duplex { + readonly chunks: Array = [] + + override _read(): void {} + + override _write( + chunk: Buffer | string, + _encoding: string, + callback: (error?: Error | null) => void + ): void { + this.chunks.push(typeof chunk === "string" ? chunk : chunk.toString("utf8")) + callback() + } +} + describe("terminal sessions service", () => { let projectRoot = "" @@ -216,6 +237,31 @@ describe("terminal sessions service", () => { rmSync(projectRoot, { force: true, recursive: true }) }) + it("denies terminal websocket upgrades when ws throws before attaching", () => { + const throwingServer: TerminalWebSocketUpgradeServer = { + handleUpgrade: () => { + throw new TypeError("upgrade requires a Request object") + } + } + const socket = new MemoryDuplexSocket() + const request = new IncomingMessage(new Socket()) + + expect(() => { + safeHandleTerminalWebSocketUpgrade( + throwingServer, + request, + socket, + Buffer.alloc(0), + () => { + throw new Error("Unexpected websocket attachment.") + } + ) + }).not.toThrow() + + expect(socket.chunks.join("")).toContain("HTTP/1.1 404 Not Found") + expect(socket.destroyed).toBe(true) + }) + it("creates a terminal session immediately when SSH is already ready", async () => { probeProjectSshReadyMock.mockImplementation(() => Effect.succeed(true)) getProjectMock.mockImplementation(() => Effect.succeed(projectDetails)) diff --git a/packages/app/scripts/serve-dist-web-forwarding.mjs b/packages/app/scripts/serve-dist-web-forwarding.mjs new file mode 100644 index 00000000..4aa74027 --- /dev/null +++ b/packages/app/scripts/serve-dist-web-forwarding.mjs @@ -0,0 +1,37 @@ +export const firstHeader = (value) => Array.isArray(value) ? value[0] : value + +const firstHeaderToken = (value) => { + const header = firstHeader(value) + const token = header?.split(",")[0]?.trim() + return token === undefined || token.length === 0 ? undefined : token +} + +const tryCloudflareHostSuffix = ".trycloudflare.com" + +const isTryCloudflareHost = (host) => { + const hostname = firstHeaderToken(host)?.split(":")[0]?.toLowerCase() + return hostname !== undefined && hostname.endsWith(tryCloudflareHostSuffix) +} + +const cfVisitorScheme = (value) => { + const scheme = /"scheme"\s*:\s*"([^"]+)"/iu.exec(firstHeader(value) ?? "")?.[1]?.toLowerCase() + return scheme === "http" || scheme === "https" ? scheme : undefined +} + +export const resolveForwardedHost = (headers) => + firstHeaderToken(headers["x-forwarded-host"]) ?? firstHeaderToken(headers.host) + +// CHANGE: Preserve HTTPS semantics for Cloudflare Quick Tunnel requests. +// WHY: Hosted Skiller Web receives a trycloudflare HTTPS backend URL and follow-up API calls must not be downgraded to http in docker-git. +// QUOTE(ТЗ): "Failed to fetch" +// REF: user-message-2026-06-18-skiller-vercel-failed-fetch +// SOURCE: n/a +// FORMAT THEOREM: host.endsWith(".trycloudflare.com") -> forwardedProto = https unless x-forwarded-proto explicitly says otherwise +// PURITY: CORE +// EFFECT: none +// INVARIANT: Explicit x-forwarded-proto has priority over Cloudflare-derived inference. +// COMPLEXITY: O(n) where n is header length. +export const resolveForwardedProto = (headers, forwardedHost = resolveForwardedHost(headers)) => + firstHeaderToken(headers["x-forwarded-proto"]) ?? + cfVisitorScheme(headers["cf-visitor"]) ?? + (isTryCloudflareHost(forwardedHost) ? "https" : "http") diff --git a/packages/app/scripts/serve-dist-web.mjs b/packages/app/scripts/serve-dist-web.mjs index 818b6f0d..ccb56111 100644 --- a/packages/app/scripts/serve-dist-web.mjs +++ b/packages/app/scripts/serve-dist-web.mjs @@ -7,6 +7,11 @@ import { fileURLToPath } from "node:url" import { WebSocket, WebSocketServer } from "ws" +import { + firstHeader, + resolveForwardedHost, + resolveForwardedProto +} from "./serve-dist-web-forwarding.mjs" import { shouldProxyHttpPath } from "./serve-dist-web-routing.mjs" const appRoot = normalize(join(fileURLToPath(new URL(".", import.meta.url)), "..")) @@ -96,11 +101,9 @@ const resolveUpstreamPath = (url) => { return `${pathname}${parsed.search}` } -const firstHeader = (value) => Array.isArray(value) ? value[0] : value - const proxyForwardHeaders = (request, forwardedPrefix) => { - const forwardedHost = firstHeader(request.headers["x-forwarded-host"]) ?? request.headers.host - const forwardedProto = firstHeader(request.headers["x-forwarded-proto"]) ?? "http" + const forwardedHost = resolveForwardedHost(request.headers) + const forwardedProto = resolveForwardedProto(request.headers, forwardedHost) return { ...request.headers, host: apiUrl.host, @@ -122,8 +125,8 @@ const parseWebSocketProtocols = (value) => { } const proxyWebSocketForwardHeaders = (request, forwardedPrefix) => { - const forwardedHost = firstHeader(request.headers["x-forwarded-host"]) ?? request.headers.host - const forwardedProto = firstHeader(request.headers["x-forwarded-proto"]) ?? "http" + const forwardedHost = resolveForwardedHost(request.headers) + const forwardedProto = resolveForwardedProto(request.headers, forwardedHost) return { ...(forwardedHost === undefined ? {} : { "x-forwarded-host": forwardedHost }), "x-forwarded-prefix": forwardedPrefix, diff --git a/packages/app/tests/docker-git/actions-skiller.test.ts b/packages/app/tests/docker-git/actions-skiller.test.ts index ba249ac2..31c13248 100644 --- a/packages/app/tests/docker-git/actions-skiller.test.ts +++ b/packages/app/tests/docker-git/actions-skiller.test.ts @@ -173,6 +173,38 @@ describe("web Skiller actions", () => { expect(openedWindow.focus).toHaveBeenCalledOnce() })) + it.effect("opens external Skiller Web launch URLs returned by the backend", () => + Effect.gen(function*(_) { + const backendUrl = ["http", "://192.168.0.206:4174/api"].join("") + const appPath = "/api/skiller/external-launch/launch-proof" + openSkillerMock.mockImplementation(() => + Effect.succeed({ + ...skillerLaunch({ + appPath, + scope: proofScope, + trpcBasePath: "https://skiller-web-henna.vercel.app/trpc" + }), + backendUrl, + mode: "external", + pid: null + }) + ) + const { context, setMessage } = makeBrowserActionContext() + + openSkillerApp(context, "abc123", "terminal-proof") + + yield* _(waitForAssertion(() => { + expect(openSkillerMock).toHaveBeenCalledWith("abc123", "terminal-proof") + })) + yield* _(waitForAssertion(() => { + expect(openedWindow.location.href).toBe(appPath) + expect(openedWindow.focus).toHaveBeenCalledOnce() + expect(setMessage).toHaveBeenCalledWith( + `Skiller Web opened. Container FS: dg-project:/home/dev/app. Opened ${appPath}.` + ) + })) + })) + it.effect("closes the prepared Skiller popup when launch fails", () => Effect.gen(function*(_) { openSkillerMock.mockImplementation(() => Effect.fail("Skiller failed")) diff --git a/packages/app/tests/docker-git/controller-resource-limits.test.ts b/packages/app/tests/docker-git/controller-resource-limits.test.ts index 2a78d0f6..a5020ff2 100644 --- a/packages/app/tests/docker-git/controller-resource-limits.test.ts +++ b/packages/app/tests/docker-git/controller-resource-limits.test.ts @@ -20,6 +20,12 @@ const composeFiles: ReadonlyArray = ["docker-compose.yml", "docker-compo const isolatedComposeFiles: ReadonlyArray = ["docker-compose.isolated.yml", "docker-compose.api.isolated.yml"] const hostDockerDataBind = "/var/lib/docker:/var/lib/docker" const isolatedDockerDataVolume = "docker_git_docker_data:/var/lib/docker" +const skillerWebEnvLines: ReadonlyArray = [ + "DOCKER_GIT_SKILLER_WEB_URL: ${DOCKER_GIT_SKILLER_WEB_URL-https://skiller-web-henna.vercel.app}", + "DOCKER_GIT_SKILLER_BACKEND_URL: ${DOCKER_GIT_SKILLER_BACKEND_URL:-}", + "DOCKER_GIT_API_PUBLIC_URL: ${DOCKER_GIT_API_PUBLIC_URL:-}", + "DOCKER_GIT_SKILLER_ALLOWED_ORIGINS: ${DOCKER_GIT_SKILLER_ALLOWED_ORIGINS:-}" +] const readComposeFile = (relativePath: string): Effect.Effect => Effect.gen(function*(_) { @@ -59,6 +65,14 @@ describe("controller compose resource limits", () => { expect(contents).toContain(`- ${hostDockerDataBind}`) expect(contents).not.toContain(`- ${isolatedDockerDataVolume}`) })) + + it.effect("passes external Skiller Web environment with empty-env opt-out", () => + Effect.gen(function*(_) { + const contents = yield* _(readComposeFile(composeFile)) + for (const line of skillerWebEnvLines) { + expect(contents).toContain(line) + } + })) }) } diff --git a/packages/app/tests/docker-git/serve-dist-web.test.ts b/packages/app/tests/docker-git/serve-dist-web.test.ts index 7d2978a4..1fa1c66e 100644 --- a/packages/app/tests/docker-git/serve-dist-web.test.ts +++ b/packages/app/tests/docker-git/serve-dist-web.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "@effect/vitest" import { Effect } from "effect" +import { resolveForwardedHost, resolveForwardedProto } from "../../scripts/serve-dist-web-forwarding.mjs" import { shouldProxyHttpPath } from "../../scripts/serve-dist-web-routing.mjs" describe("serve-dist-web routing", () => { @@ -12,4 +13,22 @@ describe("serve-dist-web routing", () => { expect(shouldProxyHttpPath("/federation/actor")).toBe(true) expect(shouldProxyHttpPath("/unknown-route")).toBe(false) })) + + it.effect("preserves HTTPS forwarding semantics for Cloudflare tunnel requests", () => + Effect.sync(() => { + expect(resolveForwardedHost({ + host: "orange-field.trycloudflare.com" + })).toBe("orange-field.trycloudflare.com") + expect(resolveForwardedProto({ + host: "orange-field.trycloudflare.com" + })).toBe("https") + expect(resolveForwardedProto({ + "cf-visitor": "{\"scheme\":\"https\"}", + host: "127.0.0.1:4174" + })).toBe("https") + expect(resolveForwardedProto({ + "x-forwarded-proto": "http", + host: "orange-field.trycloudflare.com" + })).toBe("http") + })) })