fix: harden plugin sockets, public routes, prebuilt binaries and login fallback

This commit is contained in:
LukeGus committed 2026-10-05 19:50:19 -05:00
1 parent efb4259438
commit 5e54d95e71
16 files changed
+318 -23

No files matched your search

+6 -2
View File
@@ -339,6 +339,9 @@ export interface PluginRouterOptions {
* and matched exactly ("/callback", not "/plugin-api/<id>/callback"). A
* ":name" segment matches one segment, and a trailing "/*" matches any
* number of further segments, for a proxied page ("/chooser/:id/*").
* An entry only opens the route registered with that same path, for the
* methods it was registered with, so a literal route beside a ":param"
* ("/webhook/config" next to "/webhook/:token") still needs a login.
*
* For the handful of routes an unauthenticated third party has to reach: an
* OIDC callback, an inbound webhook. Every entry is audited when the router
@@ -1859,8 +1862,9 @@ export interface PluginBinarySpec {
/** Hex SHA-256 the file must have. */
sha256: string;
/**
* Paths checked before downloading, such as a copy baked into the Docker
* image. One whose checksum matches is used in place.
* Absolute paths checked before downloading, such as a copy baked into the
* Docker image. Only a regular file named `name` is considered, and one
* whose checksum matches is used in place.
*/
prebuilt?: readonly string[];
}
+2 -1
View File
@@ -434,7 +434,8 @@ export interface PluginProtocolContribution {
defaultPort?: number;
/**
* What the owner's login falls back to from the host's own SSH login
* when the protocol login leaves it empty.
* when the protocol login leaves it empty. The password only falls back
* while the host signs in to SSH with a password.
*/
hostLoginFallback?: Array<"username" | "password">;
}
+5 -3
View File
@@ -87,7 +87,8 @@ router.get("/public", async (_req: Request, res: Response) => {
{
id: record.id,
name: record.name,
version: record.version,
// Not before login: it tells a scanner which known bugs apply.
version: "",
enabled: true,
state: loaded?.state ?? record.state,
contributes: {
@@ -121,7 +122,8 @@ router.get("/public", async (_req: Request, res: Response) => {
* that plugins provide, before anyone has signed in. Returns only
* enabled plugins that contribute login methods or second factors, with
* the fields the browser loader needs and the ids they contribute.
* Nothing operational: no capabilities, grants, settings or errors.
* Nothing operational: no versions, capabilities, grants, settings or
* errors.
* tags:
* - Plugins
* responses:
@@ -165,7 +167,7 @@ export async function listPreLoginPlugins(): Promise<
{
id: record.id,
name: record.name,
version: record.version,
version: "",
enabled: true,
state: loaded.state,
contributes: { auth: { loginMethods, secondFactors } },
@@ -511,7 +511,10 @@ export async function resolveOwnerProtocolLogin(
const fallback = declared.hostLoginFallback ?? [];
if (fallback.includes("username")) username ||= str(host.username);
if (fallback.includes("password")) password ||= str(host.password);
// A key or credential host can still carry an old password; never send it.
if (fallback.includes("password") && host.authType === "password") {
password ||= str(host.password);
}
const secretFields: Record<string, string> = {};
for (const [key, value] of Object.entries(login?.secretFields ?? {})) {
+25
View File
@@ -41,6 +41,30 @@ interface Deps {
const SHA256_PATTERN = /^[a-f0-9]{64}$/;
const SAFE_NAME = /^[A-Za-z0-9._-]+$/;
/** Larger than any CLI a plugin ships, small enough not to stall a boot. */
const MAX_PREBUILT_BYTES = 512 * 1024 * 1024;
/**
* Only a regular file carrying the binary's own name is hashed, so a plugin
* cannot use prebuilt to fingerprint arbitrary files on the server.
*/
async function isPrebuiltCandidate(
candidate: string,
name: string,
): Promise<boolean> {
if (!path.isAbsolute(candidate) || path.basename(candidate) !== name) {
return false;
}
try {
const real = await fs.realpath(candidate);
if (path.basename(real) !== name) return false;
const stat = await fs.stat(real);
return stat.isFile() && stat.size <= MAX_PREBUILT_BYTES;
} catch {
return false;
}
}
async function sha256Of(file: string): Promise<string | null> {
try {
const hash = createHash("sha256");
@@ -217,6 +241,7 @@ export function createPluginProcess(deps: Deps): PluginProcess {
}
for (const candidate of spec.prebuilt ?? []) {
if (!(await isPrebuiltCandidate(candidate, spec.name))) continue;
if ((await sha256Of(candidate)) === expected) return candidate;
}
+76 -9
View File
@@ -162,17 +162,81 @@ function compilePublicPath(path: string): RegExp {
return new RegExp(prefix ? `^${escaped}(?:/.*)?$` : `^${escaped}$`);
}
interface PublicPath {
declared: string;
pattern: RegExp;
}
interface RouterLayer {
route?: {
path?: unknown;
_handlesMethod?: (method: string) => boolean;
};
match?: (path: string) => boolean;
}
function segments(path: string): string[] {
return normalizePublicPath(path).split("/").slice(1);
}
/**
* Matches the request path against the declared public paths.
*
* Full-path match only. A prefix match would turn one declared route into a
* hole covering everything below it, which is not what declaring a single
* public path should mean.
* Whether a declared public path names this registered route: literal
* segments must be equal and a ":param" must sit where the route has one, so
* "/webhook/:token" never covers a sibling "/webhook/config".
*/
function isPublicRequest(req: Request, publicPaths: RegExp[]): boolean {
function declaredCoversRoute(declared: string, routePath: string): boolean {
const prefix = declared.endsWith("/*");
const want = segments(prefix ? declared.slice(0, -2) : declared);
const have = segments(routePath);
if (prefix ? have.length < want.length : have.length !== want.length) {
return false;
}
return want.every((segment, index) =>
segment.startsWith(":")
? have[index].startsWith(":")
: segment.toLowerCase() === have[index].toLowerCase(),
);
}
/**
* Whether the request is for a declared public path.
*
* Full-path match only, and only for the route that will actually answer:
* the first route in the plugin's router that matches the path and method
* has to be the one the declaration names. So a public "/webhook/:token"
* registered for POST does not open GET, and does not open a literal sibling
* route registered next to it. A trailing "/*" may also be served by
* router.use(), which has no route to compare against.
*/
function isPublicRequest(
req: Request,
publicPaths: PublicPath[],
inner: Router,
): boolean {
if (publicPaths.length === 0) return false;
const path = normalizePublicPath(req.path || "/");
return publicPaths.some((pattern) => pattern.test(path));
const candidates = publicPaths.filter(({ pattern }) => pattern.test(path));
if (candidates.length === 0) return false;
const stack = (inner as unknown as { stack?: RouterLayer[] }).stack ?? [];
for (const layer of stack) {
if (!layer.match?.(req.path || "/")) continue;
if (!layer.route) {
if (candidates.some(({ declared }) => declared.endsWith("/*"))) {
return true;
}
continue;
}
if (!layer.route._handlesMethod?.(req.method)) continue;
const routePath = layer.route.path;
return (
typeof routePath === "string" &&
candidates.some(({ declared }) =>
declaredCoversRoute(declared, routePath),
)
);
}
return false;
}
export interface CreatePluginRouterArgs {
@@ -196,7 +260,10 @@ export function createPluginRouter({
const declaredPublic = (options?.public ?? []).map((path) =>
normalizePublicPath(path),
);
const publicPaths = declaredPublic.map((path) => compilePublicPath(path));
const publicPaths = declaredPublic.map((declared) => ({
declared,
pattern: compilePublicPath(declared),
}));
publicRoutes.set(pluginId, declaredPublic);
if (declaredPublic.length > 0) {
@@ -250,7 +317,7 @@ export function createPluginRouter({
const authenticate = AuthManager.getInstance().createAuthMiddleware();
outer.use((req: Request, res: Response, next: NextFunction) => {
if (isPublicRequest(req, publicPaths)) {
if (isPublicRequest(req, publicPaths, inner)) {
const ip = req.ip || req.socket?.remoteAddress || "unknown";
if (overPublicRateLimit(pluginId, ip)) {
res.status(429).json({ error: "Too many requests" });
+10 -1
View File
@@ -28,7 +28,10 @@ import type {
} from "@termix-ssh/plugin-sdk/backend";
import { pluginLogger } from "../utils/logger.js";
import { recordConflict } from "./conflicts.js";
import { extractWebSocketToken } from "../utils/ws-auth.js";
import {
extractWebSocketToken,
isCookieOriginAllowed,
} from "../utils/ws-auth.js";
import { runAsActor } from "./actor.js";
import { isPluginInstalled } from "./http.js";
@@ -163,6 +166,12 @@ async function handlePluginUpgrade(
return true;
}
// A page on another site gets the browser's cookies attached for free.
// Core already ignores them for auth; plugin code must not see them either.
if (request.headers.cookie && !isCookieOriginAllowed(request)) {
delete request.headers.cookie;
}
let userId = "";
if (!route.options.public) {
const resolved = await verifyToken(request);
@@ -9,7 +9,10 @@
import { asc } from "drizzle-orm";
import { afterEach, describe, expect, it } from "vitest";
import { TestSqliteDatabase } from "./repositories/test-support.js";
import {
TestSqliteDatabase,
testDialect,
} from "./repositories/test-support.js";
import { HostRepository } from "../../database/repositories/host-repository.js";
import { UserRepository } from "../../database/repositories/user-repository.js";
import type { DatabaseContext } from "../../database/repositories/database-context.js";
@@ -48,13 +51,16 @@ async function seed(): Promise<DatabaseContext> {
return context;
}
// key is reserved in MySQL
const KEY_COLUMN = testDialect() === "mysql" ? "`key`" : "key";
async function addSetting(
scope: string,
scopeId: string | null,
key = "k",
): Promise<void> {
await adapter!.exec(
`INSERT INTO plugin_settings (plugin_id, scope, scope_id, key, value)
`INSERT INTO plugin_settings (plugin_id, scope, scope_id, ${KEY_COLUMN}, value)
VALUES ('sample', '${scope}', ${scopeId === null ? "NULL" : `'${scopeId}'`}, '${key}', '"v"')`,
);
}
@@ -239,6 +239,7 @@ describe("plugins route", () => {
});
expect(body[0]).not.toHaveProperty("capabilities");
expect(body[0]).not.toHaveProperty("lastError");
expect(body[0].version).toBe("");
});
it("skips a plugin whose manifest does not parse", async () => {
@@ -314,6 +315,9 @@ describe("plugins route", () => {
expect(body[0].contributes).toEqual({
auth: { loginMethods: ["corp-sso"], secondFactors: [] },
});
expect(body.map((plugin: { version: string }) => plugin.version)).toEqual(
["", ""],
);
for (const plugin of body) {
expect(Object.keys(plugin).sort()).toEqual(
[
@@ -156,6 +156,7 @@ const baseHost = {
ip: "10.0.0.7",
port: 22,
username: "sshuser",
authType: "password",
password: "ssh-secret",
jumpHosts: JSON.stringify([{ hostId: 3 }]),
};
@@ -285,6 +286,26 @@ describe("ctx.credentials.resolveHostProtocol", () => {
});
});
it("does not fall back to an old password on a key host", async () => {
grants.set("demo", ["credentials:read"]);
state.hosts.set(7, { ...baseHost, authType: "key" });
state.logins.set("7:spice", {
protocol: "spice",
authType: "direct",
credentialId: null,
username: null,
password: null,
fields: {},
secretFields: {},
});
const ctx = spiceContext(["credentials:read"]);
const target = await runAsActor("owner", "request", () =>
ctx.credentials.resolveHostProtocol(7, "spice"),
);
expect(target?.auth.password).toBe("");
});
it("uses the stored credential in credential mode", async () => {
grants.set("demo", ["credentials:read"]);
state.logins.set("7:spice", {
+19 -1
View File
@@ -113,7 +113,8 @@ describe("ctx.process.ensureBinary", () => {
const contents = "#!/bin/sh\necho hi\n";
it("uses a prebuilt copy whose checksum matches, without downloading", async () => {
const prebuilt = path.join(dir, "prebuilt-tool");
await fs.mkdir(path.join(dir, "prebuilt"), { recursive: true });
const prebuilt = path.join(dir, "prebuilt", "tool");
await fs.writeFile(prebuilt, contents);
const fetch = vi.fn();
const { process: proc } = create({ fetch: fetch as never });
@@ -129,6 +130,23 @@ describe("ctx.process.ensureBinary", () => {
expect(fetch).not.toHaveBeenCalled();
});
it("never hashes a prebuilt path that is not the binary itself", async () => {
const other = path.join(dir, "secrets.db");
await fs.writeFile(other, contents);
await fs.mkdir(path.join(dir, "tool-dir", "tool"), { recursive: true });
const fetch = vi.fn(async () => new Response(contents));
const { process: proc } = create({ fetch: fetch as never });
const result = await proc.ensureBinary({
name: "tool",
version: "1",
url: "https://example.test/tool",
sha256: sha256(contents),
prebuilt: [other, "relative/tool", path.join(dir, "tool-dir", "tool")],
});
expect(result).toBe(path.join(dir, "bin", "tool"));
expect(fetch).toHaveBeenCalledOnce();
});
it("downloads once, verifies it and reuses the copy", async () => {
const fetch = vi.fn(async () => new Response(contents));
const { process: proc } = create({ fetch: fetch as never });
@@ -33,7 +33,19 @@ vi.mock("../../plugins/permissions.js", () => ({
invalidatePluginPermissionCache: vi.fn(),
}));
const TEST_PORT = 39102;
// A fixed port can land in a range Windows reserves for Hyper-V or Docker.
function freePort(): Promise<number> {
return new Promise((resolve, reject) => {
const server = net.createServer();
server.once("error", reject);
server.listen(0, "127.0.0.1", () => {
const { port } = server.address() as net.AddressInfo;
server.close(() => resolve(port));
});
});
}
const TEST_PORT = await freePort();
function isListening(port: number): Promise<boolean> {
return new Promise((resolve) => {
+42
View File
@@ -234,6 +234,48 @@ describe("ctx.http authentication", () => {
expect(response.status).toBe(401);
});
it("only opens the methods the public route was registered for", async () => {
const router = http.createPluginRouter({
manifest: manifest(),
options: { public: ["/webhook/:token"] },
reportError: () => {},
});
router.post("/webhook/:token", (_req, res) => res.json({ ok: true }));
router.delete("/webhook/:token", (_req, res) => res.json({ ok: true }));
const base = await startServer();
const url = `${base}/plugin-api/sample-plugin/webhook/abc`;
expect((await fetch(url, { method: "POST" })).status).toBe(200);
expect((await fetch(url)).status).toBe(401);
expect((await fetch(url, { method: "PUT" })).status).toBe(401);
});
it("does not open a literal route beside a public parameter", async () => {
const router = http.createPluginRouter({
manifest: manifest(),
options: { public: ["/webhook/:token"] },
reportError: () => {},
});
router.get("/webhook/config", (_req, res) => res.json({ secret: true }));
router.post("/webhook/config", (_req, res) => res.json({ secret: true }));
router.get("/webhook/:token", (_req, res) => res.json({ ok: true }));
router.post("/webhook/:token", (_req, res) => res.json({ ok: true }));
const base = await startServer();
const root = `${base}/plugin-api/sample-plugin/webhook`;
expect((await fetch(`${root}/config`)).status).toBe(401);
expect((await fetch(`${root}/config`, { method: "POST" })).status).toBe(
401,
);
expect((await fetch(`${root}/abc`)).status).toBe(200);
expect((await fetch(`${root}/abc`, { method: "HEAD" })).status).toBe(200);
expect((await fetch(`${root}/config`, { headers: authed })).status).toBe(
200,
);
});
it("opens everything below a trailing /* and nothing beside it", async () => {
const router = http.createPluginRouter({
manifest: manifest(),
+13 -1
View File
@@ -34,7 +34,19 @@ vi.mock("../../plugins/permissions.js", () => ({
invalidatePluginPermissionCache: vi.fn(),
}));
const CONSOLE_PORT = 39109;
// A fixed port can land in a range Windows reserves for Hyper-V or Docker.
function freePort(): Promise<number> {
return new Promise((resolve, reject) => {
const server = net.createServer();
server.once("error", reject);
server.listen(0, "127.0.0.1", () => {
const { port } = server.address() as net.AddressInfo;
server.close(() => resolve(port));
});
});
}
const CONSOLE_PORT = await freePort();
const cleanups: Array<() => void> = [];
function tempRoot(prefix: string): string {
+69
View File
@@ -91,6 +91,20 @@ function connect(
});
}
function connectWithHeaders(
url: string,
headers: Record<string, string>,
): Promise<{ outcome: "open"; socket: WebSocket } | { outcome: number }> {
return new Promise((resolve) => {
const socket = new WebSocket(url, { headers });
socket.once("open", () => resolve({ outcome: "open", socket }));
socket.once("unexpected-response", (_req, res) =>
resolve({ outcome: res.statusCode ?? 0 }),
);
socket.once("error", () => resolve({ outcome: 0 }));
});
}
function authProtocols(token: string): string[] {
return [`termix.jwt.${token}`];
}
@@ -159,6 +173,61 @@ describe("ctx.ws routing", () => {
});
describe("ctx.ws authentication", () => {
it("ignores the jwt cookie on an upgrade from another site", async () => {
ws.registerPluginWsRoute("sample-plugin", "/socket", () => {}, DECLARED);
const base = await startServer();
const result = await connectWithHeaders(
`${base}/plugin-ws/sample-plugin/socket`,
{ Cookie: `jwt=${state.validToken}`, Origin: "https://evil.example" },
);
expect(result.outcome).toBe(401);
});
it("accepts the jwt cookie from this server's own page", async () => {
ws.registerPluginWsRoute("sample-plugin", "/socket", () => {}, DECLARED);
const base = await startServer();
const result = await connectWithHeaders(
`${base}/plugin-ws/sample-plugin/socket`,
{
Cookie: `jwt=${state.validToken}`,
Origin: base.replace(/^ws:/, "http:"),
},
);
expect(result.outcome).toBe("open");
if (result.outcome === "open") result.socket.close();
});
it("hides cross-site cookies from a public socket's handler", async () => {
const cookies: Array<string | undefined> = [];
const users: string[] = [];
ws.registerPluginWsRoute(
"sample-plugin",
"/public",
({ request, userId, socket }) => {
cookies.push(request.headers.cookie);
users.push(userId);
(socket as WebSocket).close();
},
DECLARED,
{ public: true, optionalAuth: true },
);
const base = await startServer();
const result = await connectWithHeaders(
`${base}/plugin-ws/sample-plugin/public`,
{ Cookie: `jwt=${state.validToken}`, Origin: "https://evil.example" },
);
expect(result.outcome).toBe("open");
await vi.waitFor(() => expect(users).toHaveLength(1));
expect(users).toEqual([""]);
expect(cookies).toEqual([undefined]);
});
it("rejects an upgrade with no token", async () => {
ws.registerPluginWsRoute("sample-plugin", "/socket", () => {}, DECLARED);
@@ -195,7 +195,7 @@ describe("runProtocolAuthMigration", () => {
username: "waits",
});
const [marker] = await database!.query<{ value: string }>(
sql`SELECT value FROM settings WHERE key = ${copiedMarker("locked")}`,
sql`SELECT value FROM settings WHERE ${sql.identifier("key")} = ${copiedMarker("locked")}`,
);
expect(marker.value).toBe("done");
});