refactor: tighten external-link policy and window.open guard

This commit is contained in:
Peter Steinberger
2026-02-24 15:05:09 +00:00
parent 13bfe7faa6
commit 6c5ab543c0
8 changed files with 162 additions and 29 deletions

View File

@@ -3,6 +3,7 @@
import { promises as fs } from "node:fs"; import { promises as fs } from "node:fs";
import path from "node:path"; import path from "node:path";
import { fileURLToPath } from "node:url"; import { fileURLToPath } from "node:url";
import ts from "typescript";
const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..");
const uiSourceDir = path.join(repoRoot, "ui", "src", "ui"); const uiSourceDir = path.join(repoRoot, "ui", "src", "ui");
@@ -39,20 +40,67 @@ async function collectTypeScriptFiles(dir) {
return out; return out;
} }
function lineNumberAt(content, index) { function unwrapExpression(expression) {
let lines = 1; let current = expression;
for (let i = 0; i < index; i++) { while (true) {
if (content.charCodeAt(i) === 10) { if (ts.isParenthesizedExpression(current)) {
lines++; current = current.expression;
continue;
} }
if (ts.isAsExpression(current) || ts.isTypeAssertionExpression(current)) {
current = current.expression;
continue;
}
if (ts.isNonNullExpression(current)) {
current = current.expression;
continue;
}
return current;
} }
}
function asPropertyAccess(expression) {
if (ts.isPropertyAccessExpression(expression)) {
return expression;
}
if (typeof ts.isPropertyAccessChain === "function" && ts.isPropertyAccessChain(expression)) {
return expression;
}
return null;
}
function isRawWindowOpenCall(expression) {
const propertyAccess = asPropertyAccess(unwrapExpression(expression));
if (!propertyAccess || propertyAccess.name.text !== "open") {
return false;
}
const receiver = unwrapExpression(propertyAccess.expression);
return (
ts.isIdentifier(receiver) && (receiver.text === "window" || receiver.text === "globalThis")
);
}
export function findRawWindowOpenLines(content, fileName = "source.ts") {
const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.Latest, true);
const lines = [];
const visit = (node) => {
if (ts.isCallExpression(node) && isRawWindowOpenCall(node.expression)) {
const line =
sourceFile.getLineAndCharacterOfPosition(node.expression.getStart(sourceFile)).line + 1;
lines.push(line);
}
ts.forEachChild(node, visit);
};
visit(sourceFile);
return lines; return lines;
} }
async function main() { export async function main() {
const files = await collectTypeScriptFiles(uiSourceDir); const files = await collectTypeScriptFiles(uiSourceDir);
const violations = []; const violations = [];
const rawWindowOpenRe = /\bwindow\s*\.\s*open\s*\(/g;
for (const filePath of files) { for (const filePath of files) {
if (allowedCallsites.has(filePath)) { if (allowedCallsites.has(filePath)) {
@@ -60,12 +108,9 @@ async function main() {
} }
const content = await fs.readFile(filePath, "utf8"); const content = await fs.readFile(filePath, "utf8");
let match = rawWindowOpenRe.exec(content); for (const line of findRawWindowOpenLines(content, filePath)) {
while (match) {
const line = lineNumberAt(content, match.index);
const relPath = path.relative(repoRoot, filePath); const relPath = path.relative(repoRoot, filePath);
violations.push(`${relPath}:${line}`); violations.push(`${relPath}:${line}`);
match = rawWindowOpenRe.exec(content);
} }
} }
@@ -81,7 +126,17 @@ async function main() {
process.exit(1); process.exit(1);
} }
main().catch((error) => { const isDirectExecution = (() => {
console.error(error); const entry = process.argv[1];
process.exit(1); if (!entry) {
}); return false;
}
return path.resolve(entry) === fileURLToPath(import.meta.url);
})();
if (isDirectExecution) {
main().catch((error) => {
console.error(error);
process.exit(1);
});
}

View File

@@ -0,0 +1,39 @@
import { describe, expect, it } from "vitest";
import { findRawWindowOpenLines } from "../../scripts/check-no-raw-window-open.mjs";
describe("check-no-raw-window-open", () => {
it("finds direct window.open calls", () => {
const source = `
function openDocs() {
window.open("https://docs.openclaw.ai");
}
`;
expect(findRawWindowOpenLines(source)).toEqual([3]);
});
it("finds globalThis.open calls", () => {
const source = `
function openDocs() {
globalThis.open("https://docs.openclaw.ai");
}
`;
expect(findRawWindowOpenLines(source)).toEqual([3]);
});
it("ignores mentions in strings and comments", () => {
const source = `
// window.open("https://example.com")
const text = "window.open('https://example.com')";
`;
expect(findRawWindowOpenLines(source)).toEqual([]);
});
it("handles parenthesized and asserted window references", () => {
const source = `
const openRef = (window as Window).open;
openRef("https://example.com");
(window as Window).open("https://example.com");
`;
expect(findRawWindowOpenLines(source)).toEqual([4]);
});
});

View File

@@ -62,6 +62,7 @@ import {
updateSkillEdit, updateSkillEdit,
updateSkillEnabled, updateSkillEnabled,
} from "./controllers/skills.ts"; } from "./controllers/skills.ts";
import { buildExternalLinkRel, EXTERNAL_LINK_TARGET } from "./external-link.ts";
import { icons } from "./icons.ts"; import { icons } from "./icons.ts";
import { normalizeBasePath, TAB_GROUPS, subtitleForTab, titleForTab } from "./navigation.ts"; import { normalizeBasePath, TAB_GROUPS, subtitleForTab, titleForTab } from "./navigation.ts";
import { renderAgents } from "./views/agents.ts"; import { renderAgents } from "./views/agents.ts";
@@ -289,8 +290,8 @@ export function renderApp(state: AppViewState) {
<a <a
class="nav-item nav-item--external" class="nav-item nav-item--external"
href="https://docs.openclaw.ai" href="https://docs.openclaw.ai"
target="_blank" target=${EXTERNAL_LINK_TARGET}
rel="noreferrer" rel=${buildExternalLinkRel()}
title="${t("common.docs")} (opens in new tab)" title="${t("common.docs")} (opens in new tab)"
> >
<span class="nav-item__icon" aria-hidden="true">${icons.book}</span> <span class="nav-item__icon" aria-hidden="true">${icons.book}</span>

View File

@@ -0,0 +1,18 @@
import { describe, expect, it } from "vitest";
import { buildExternalLinkRel } from "./external-link.ts";
describe("buildExternalLinkRel", () => {
it("always includes required security tokens", () => {
expect(buildExternalLinkRel()).toBe("noopener noreferrer");
});
it("preserves extra rel tokens while deduping required ones", () => {
expect(buildExternalLinkRel("noreferrer nofollow NOOPENER")).toBe(
"noopener noreferrer nofollow",
);
});
it("ignores whitespace-only rel input", () => {
expect(buildExternalLinkRel(" ")).toBe("noopener noreferrer");
});
});

View File

@@ -0,0 +1,19 @@
const REQUIRED_EXTERNAL_REL_TOKENS = ["noopener", "noreferrer"] as const;
export const EXTERNAL_LINK_TARGET = "_blank";
export function buildExternalLinkRel(currentRel?: string): string {
const extraTokens: string[] = [];
const seen = new Set(REQUIRED_EXTERNAL_REL_TOKENS);
for (const rawToken of (currentRel ?? "").split(/\s+/)) {
const token = rawToken.trim().toLowerCase();
if (!token || seen.has(token)) {
continue;
}
seen.add(token);
extraTokens.push(token);
}
return [...REQUIRED_EXTERNAL_REL_TOKENS, ...extraTokens].join(" ");
}

View File

@@ -1,5 +1,6 @@
import { afterEach, beforeEach } from "vitest"; import { afterEach, beforeEach } from "vitest";
import { OpenClawApp } from "../app.ts"; import "../app.ts";
import type { OpenClawApp } from "../app.ts";
export function mountApp(pathname: string) { export function mountApp(pathname: string) {
window.history.replaceState({}, "", pathname); window.history.replaceState({}, "", pathname);

View File

@@ -1,5 +1,4 @@
import { afterEach, describe, expect, it, vi } from "vitest"; import { afterEach, describe, expect, it, vi } from "vitest";
import "../app.ts";
import { mountApp, registerAppMountHooks } from "../test-helpers/app-mount.ts"; import { mountApp, registerAppMountHooks } from "../test-helpers/app-mount.ts";
registerAppMountHooks(); registerAppMountHooks();

View File

@@ -1,6 +1,7 @@
import { html } from "lit"; import { html } from "lit";
import { ConnectErrorDetailCodes } from "../../../../src/gateway/protocol/connect-error-details.js"; import { ConnectErrorDetailCodes } from "../../../../src/gateway/protocol/connect-error-details.js";
import { t, i18n, type Locale } from "../../i18n/index.ts"; import { t, i18n, type Locale } from "../../i18n/index.ts";
import { buildExternalLinkRel, EXTERNAL_LINK_TARGET } from "../external-link.ts";
import { formatRelativeTimestamp, formatDurationHuman } from "../format.ts"; import { formatRelativeTimestamp, formatDurationHuman } from "../format.ts";
import type { GatewayHelloOk } from "../gateway.ts"; import type { GatewayHelloOk } from "../gateway.ts";
import { formatNextRun } from "../presenter.ts"; import { formatNextRun } from "../presenter.ts";
@@ -59,8 +60,8 @@ export function renderOverview(props: OverviewProps) {
<a <a
class="session-link" class="session-link"
href="https://docs.openclaw.ai/web/control-ui#device-pairing-first-connection" href="https://docs.openclaw.ai/web/control-ui#device-pairing-first-connection"
target="_blank" target=${EXTERNAL_LINK_TARGET}
rel="noreferrer" rel=${buildExternalLinkRel()}
title="Device pairing docs (opens in new tab)" title="Device pairing docs (opens in new tab)"
>Docs: Device pairing</a >Docs: Device pairing</a
> >
@@ -116,8 +117,8 @@ export function renderOverview(props: OverviewProps) {
<a <a
class="session-link" class="session-link"
href="https://docs.openclaw.ai/web/dashboard" href="https://docs.openclaw.ai/web/dashboard"
target="_blank" target=${EXTERNAL_LINK_TARGET}
rel="noreferrer" rel=${buildExternalLinkRel()}
title="Control UI auth docs (opens in new tab)" title="Control UI auth docs (opens in new tab)"
>Docs: Control UI auth</a >Docs: Control UI auth</a
> >
@@ -132,8 +133,8 @@ export function renderOverview(props: OverviewProps) {
<a <a
class="session-link" class="session-link"
href="https://docs.openclaw.ai/web/dashboard" href="https://docs.openclaw.ai/web/dashboard"
target="_blank" target=${EXTERNAL_LINK_TARGET}
rel="noreferrer" rel=${buildExternalLinkRel()}
title="Control UI auth docs (opens in new tab)" title="Control UI auth docs (opens in new tab)"
>Docs: Control UI auth</a >Docs: Control UI auth</a
> >
@@ -171,8 +172,8 @@ export function renderOverview(props: OverviewProps) {
<a <a
class="session-link" class="session-link"
href="https://docs.openclaw.ai/gateway/tailscale" href="https://docs.openclaw.ai/gateway/tailscale"
target="_blank" target=${EXTERNAL_LINK_TARGET}
rel="noreferrer" rel=${buildExternalLinkRel()}
title="Tailscale Serve docs (opens in new tab)" title="Tailscale Serve docs (opens in new tab)"
>Docs: Tailscale Serve</a >Docs: Tailscale Serve</a
> >
@@ -180,8 +181,8 @@ export function renderOverview(props: OverviewProps) {
<a <a
class="session-link" class="session-link"
href="https://docs.openclaw.ai/web/control-ui#insecure-http" href="https://docs.openclaw.ai/web/control-ui#insecure-http"
target="_blank" target=${EXTERNAL_LINK_TARGET}
rel="noreferrer" rel=${buildExternalLinkRel()}
title="Insecure HTTP docs (opens in new tab)" title="Insecure HTTP docs (opens in new tab)"
>Docs: Insecure HTTP</a >Docs: Insecure HTTP</a
> >