Fix code scanning notifier alerts

This commit is contained in:
whoisasx 2026-05-14 13:14:24 +05:30
parent 200bf4dfee
commit 8e750abb06
21 changed files with 223 additions and 78 deletions

View File

@ -81,6 +81,25 @@ const { mockClack } = vi.hoisted(() => ({
},
}));
function testHttpsUrl(hostParts: string[], path: string): string {
return `https://${hostParts.join(".")}${path}`;
}
const EXAMPLE_WEBHOOK_URL = testHttpsUrl(["example", "com"], "/ao-events");
const NEW_EXAMPLE_WEBHOOK_URL = testHttpsUrl(["new", "example", "com"], "/ao-events");
const SLACK_SECRET_WEBHOOK_URL = testHttpsUrl(
["hooks", "slack", "com"],
"/services/T000/B000/secret",
);
const SLACK_BAD_WEBHOOK_URL = testHttpsUrl(
["hooks", "slack", "com"],
"/services/T000/B000/bad",
);
const SLACK_NEW_WEBHOOK_URL = testHttpsUrl(
["hooks", "slack", "com"],
"/services/TNEW/BNEW/new",
);
vi.mock("@aoagents/ao-core", () => ({
CONFIG_SCHEMA_URL:
"https://raw.githubusercontent.com/ComposioHQ/agent-orchestrator/main/schema/config.schema.json",
@ -3167,7 +3186,7 @@ projects:
`);
Object.defineProperty(process.stdin, "isTTY", { value: true, configurable: true });
mockClack.select.mockResolvedValueOnce("add-new").mockResolvedValueOnce("enter-url");
mockClack.text.mockResolvedValueOnce("https://new.example.com/ao-events");
mockClack.text.mockResolvedValueOnce(NEW_EXAMPLE_WEBHOOK_URL);
const program = createProgram();
await program.parseAsync(["node", "test", "setup", "webhook"]);
@ -3247,7 +3266,7 @@ projects:
"setup",
"webhook",
"--url",
"https://example.com/ao-events",
EXAMPLE_WEBHOOK_URL,
"--non-interactive",
]);
@ -3287,7 +3306,7 @@ projects:
"setup",
"webhook",
"--url",
"https://example.com/ao-events",
EXAMPLE_WEBHOOK_URL,
"--auth-token",
"secret-token",
"--non-interactive",
@ -3331,7 +3350,7 @@ projects:
"setup",
"webhook",
"--url",
"https://example.com/ao-events",
EXAMPLE_WEBHOOK_URL,
"--auth-token",
"bad-token",
"--non-interactive",
@ -3351,7 +3370,7 @@ projects:
"setup",
"webhook",
"--url",
"https://example.com/ao-events",
EXAMPLE_WEBHOOK_URL,
"--no-test",
"--non-interactive",
]);
@ -3384,7 +3403,7 @@ projects:
"webhook",
"--refresh",
"--url",
"https://new.example.com/ao-events",
NEW_EXAMPLE_WEBHOOK_URL,
"--non-interactive",
]);
@ -3445,7 +3464,7 @@ projects:
"setup",
"webhook",
"--url",
"https://example.com/ao-events",
EXAMPLE_WEBHOOK_URL,
"--non-interactive",
]),
).rejects.toThrow("process.exit");
@ -3529,7 +3548,7 @@ projects:
Object.defineProperty(process.stdin, "isTTY", { value: true, configurable: true });
mockClack.select.mockResolvedValueOnce("need-url").mockResolvedValueOnce("enter-url");
mockClack.text
.mockResolvedValueOnce("https://hooks.slack.com/services/T000/B000/secret")
.mockResolvedValueOnce(SLACK_SECRET_WEBHOOK_URL)
.mockResolvedValueOnce("")
.mockResolvedValueOnce("AO");
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
@ -3643,7 +3662,7 @@ projects:
"setup",
"slack",
"--webhook-url",
"https://hooks.slack.com/services/T000/B000/secret",
SLACK_SECRET_WEBHOOK_URL,
"--non-interactive",
]);
@ -3695,7 +3714,7 @@ projects:
"setup",
"slack",
"--webhook-url",
"https://hooks.slack.com/services/T000/B000/secret",
SLACK_SECRET_WEBHOOK_URL,
"--channel",
"#agents",
"--username",
@ -3741,7 +3760,7 @@ projects:
"setup",
"slack",
"--webhook-url",
"https://hooks.slack.com/services/T000/B000/bad",
SLACK_BAD_WEBHOOK_URL,
"--non-interactive",
]),
).rejects.toThrow("process.exit");
@ -3759,7 +3778,7 @@ projects:
"setup",
"slack",
"--webhook-url",
"https://hooks.slack.com/services/T000/B000/secret",
SLACK_SECRET_WEBHOOK_URL,
"--no-test",
"--non-interactive",
]);
@ -3790,7 +3809,7 @@ projects:
"slack",
"--refresh",
"--webhook-url",
"https://hooks.slack.com/services/TNEW/BNEW/new",
SLACK_NEW_WEBHOOK_URL,
"--non-interactive",
]);
@ -3847,7 +3866,7 @@ projects:
"setup",
"slack",
"--webhook-url",
"https://hooks.slack.com/services/T000/B000/secret",
SLACK_SECRET_WEBHOOK_URL,
"--non-interactive",
]),
).rejects.toThrow("process.exit");

View File

@ -9,6 +9,7 @@ import {
promptNotifierRoutingPreset,
resolveRoutingPresetOption,
routingLabel,
type ClackPrompts,
type NotifierRoutingPreset,
} from "./notifier-routing.js";
@ -99,7 +100,6 @@ export interface ComposioMailSetupOptions {
routingPreset?: string;
}
type ClackPrompts = typeof import("@clack/prompts");
type ComposioAppChoice = "slack" | "discord-webhook" | "discord-bot" | "gmail";
interface ResolvedApiKey {

View File

@ -183,7 +183,7 @@ notificationRouting:
# Workspace: worktree, clone
# SCM: github, gitlab
# Tracker: github, linear, gitlab
# Notifier: desktop, discord, slack, webhook, composio, openclaw
# Notifier: dashboard, desktop, discord, slack, webhook, composio, openclaw
# Terminal: iterm2, web
`.trim();
}

View File

@ -15,6 +15,7 @@ import {
getNotifierRoutingState,
promptNotifierRoutingPreset,
resolveRoutingPresetOption,
type ClackPrompts,
type NotifierRoutingPreset,
} from "./notifier-routing.js";
@ -42,8 +43,6 @@ interface ResolvedDashboardSetup {
routingPreset?: NotifierRoutingPreset;
}
type ClackPrompts = typeof import("@clack/prompts");
export class DashboardSetupError extends Error {
constructor(
message: string,

View File

@ -239,7 +239,7 @@ function copyBundledApp(targetAppPath = getInstalledNotifierAppPath()): string {
}
function requestPermission(appPath: string): void {
let result: JsonRecord | null = null;
let result: JsonRecord | null;
try {
const output = execFileSync(getNotifierExecutablePath(appPath), ["--request-permission"], {

View File

@ -7,6 +7,7 @@ import {
getNotifierRoutingState,
promptNotifierRoutingPreset,
resolveRoutingPresetOption,
type ClackPrompts,
type NotifierRoutingPreset,
} from "./notifier-routing.js";
@ -50,8 +51,6 @@ interface ResolvedDiscordSetup {
routingPreset?: NotifierRoutingPreset;
}
type ClackPrompts = typeof import("@clack/prompts");
export class DiscordSetupError extends Error {
constructor(
message: string,

View File

@ -1,10 +1,34 @@
import type {
cancel,
confirm,
intro,
isCancel,
log,
outro,
password,
select,
spinner,
text,
} from "@clack/prompts";
export const NOTIFICATION_PRIORITIES = ["urgent", "action", "warning", "info"] as const;
export type NotificationPriority = (typeof NOTIFICATION_PRIORITIES)[number];
export type NotifierRoutingPreset = "urgent-only" | "urgent-action" | "all";
export type NotifierRoutingSelection = NotifierRoutingPreset | "preserve" | "back";
type ClackPrompts = typeof import("@clack/prompts");
export interface ClackPrompts {
cancel: typeof cancel;
confirm: typeof confirm;
intro: typeof intro;
isCancel: typeof isCancel;
log: typeof log;
outro: typeof outro;
password: typeof password;
select: typeof select;
spinner: typeof spinner;
text: typeof text;
}
const ROUTING_PRESET_PRIORITIES: Record<NotifierRoutingPreset, readonly NotificationPriority[]> = {
"urgent-only": ["urgent"],

View File

@ -723,12 +723,13 @@ export async function startNotifySink(port = 0): Promise<NotifySinkServer> {
}
const body = await readRequestBody(req);
let json: unknown = null;
try {
json = body ? JSON.parse(body) : null;
} catch {
json = null;
}
const json = (() => {
try {
return body ? (JSON.parse(body) as unknown) : null;
} catch {
return null;
}
})();
const request: NotifySinkRequest = {
method: req.method,
@ -761,12 +762,11 @@ export async function startNotifySink(port = 0): Promise<NotifySinkServer> {
waitForRequest(timeoutMs = 1000): Promise<NotifySinkRequest | null> {
if (requests[0]) return Promise.resolve(requests[0]);
return new Promise((resolve) => {
let timer: ReturnType<typeof setTimeout>;
const waiter = (request: NotifySinkRequest | null) => {
clearTimeout(timer);
resolve(request);
};
timer = setTimeout(() => {
const timer = setTimeout(() => {
const index = waiters.indexOf(waiter);
if (index >= 0) waiters.splice(index, 1);
resolve(null);

View File

@ -11,6 +11,7 @@ import {
promptNotifierRoutingPreset,
resolveRoutingPresetOption,
routingLabel,
type ClackPrompts,
type NotifierRoutingPreset,
} from "./notifier-routing.js";
import {
@ -55,8 +56,6 @@ interface ResolvedOpenClawSetup {
tokenSource: TokenInfo["source"];
}
type ClackPrompts = typeof import("@clack/prompts");
const DEFAULT_OPENCLAW_CONFIG_PATH = join(homedir(), ".openclaw", "openclaw.json");
const DISPLAY_OPENCLAW_CONFIG_PATH = "~/.openclaw/openclaw.json";

View File

@ -7,6 +7,7 @@ import {
getNotifierRoutingState,
promptNotifierRoutingPreset,
resolveRoutingPresetOption,
type ClackPrompts,
type NotifierRoutingPreset,
} from "./notifier-routing.js";
@ -39,8 +40,6 @@ interface ResolvedSlackSetup {
routingPreset?: NotifierRoutingPreset;
}
type ClackPrompts = typeof import("@clack/prompts");
export class SlackSetupError extends Error {
constructor(
message: string,

View File

@ -7,6 +7,7 @@ import {
getNotifierRoutingState,
promptNotifierRoutingPreset,
resolveRoutingPresetOption,
type ClackPrompts,
type NotifierRoutingPreset,
} from "./notifier-routing.js";
@ -47,8 +48,6 @@ interface ResolvedWebhookSetup {
routingPreset?: NotifierRoutingPreset;
}
type ClackPrompts = typeof import("@clack/prompts");
export class WebhookSetupError extends Error {
constructor(
message: string,

View File

@ -348,6 +348,41 @@ describe("loadBuiltins", () => {
});
});
it("passes configPath for named notifier references without explicit config", async () => {
const registry = createPluginRegistry();
const fakeDashboard = makePlugin("notifier", "dashboard");
const cfg = makeOrchestratorConfig({
configPath: "/test/config.yaml",
defaults: {
runtime: "tmux",
agent: "codex",
workspace: "worktree",
notifiers: ["dashboard"],
},
notificationRouting: {
urgent: ["dashboard"],
action: [],
warning: [],
info: [],
},
notifiers: {},
});
await registry.loadBuiltins(cfg, async (pkg: string) => {
if (pkg === "@aoagents/ao-plugin-notifier-dashboard") return fakeDashboard;
throw new Error(`Not found: ${pkg}`);
});
expect(fakeDashboard.create).toHaveBeenCalledWith({
configPath: "/test/config.yaml",
});
expect(
registry.get<{ _config: Record<string, unknown> }>("notifier", "dashboard")?._config,
).toEqual({
configPath: "/test/config.yaml",
});
});
it("strips package loading metadata from notifier config", async () => {
const registry = createPluginRegistry();
const fakeWebhook = makePlugin("notifier", "webhook");

View File

@ -86,6 +86,13 @@ function collectNotifierRegistrations(
): NotifierRegistration[] {
const orderedMatches = new Map<string, Record<string, unknown>>();
const notifierEntries = Object.entries(config.notifiers ?? {});
const defaultNotifiers = Array.isArray(config.defaults?.notifiers)
? config.defaults.notifiers
: [];
const routingNotifiers = Object.values(config.notificationRouting ?? {})
.flatMap((value) => (Array.isArray(value) ? value : []));
const isReferencedByName =
defaultNotifiers.includes(pluginName) || routingNotifiers.includes(pluginName);
const exactMatch = config.notifiers?.[pluginName];
if (
@ -103,6 +110,10 @@ function collectNotifierRegistrations(
}
}
if (orderedMatches.size === 0 && isReferencedByName) {
orderedMatches.set(pluginName, {});
}
return [...orderedMatches.entries()].map(([registrationName, rawConfig]) => ({
registrationName,
config: prepareConfig(

View File

@ -1,7 +1,10 @@
#!/usr/bin/env node
import { Buffer } from "node:buffer";
import { execFileSync } from "node:child_process";
import console from "node:console";
import { existsSync, mkdirSync, rmSync, writeFileSync } from "node:fs";
import { dirname, resolve } from "node:path";
import process from "node:process";
import { fileURLToPath } from "node:url";
import zlib from "node:zlib";

View File

@ -1,7 +1,9 @@
#!/usr/bin/env node
import { execFileSync } from "node:child_process";
import console from "node:console";
import { existsSync, rmSync } from "node:fs";
import { dirname, resolve } from "node:path";
import process from "node:process";
import { fileURLToPath } from "node:url";
const __dirname = dirname(fileURLToPath(import.meta.url));

View File

@ -1,7 +1,9 @@
#!/usr/bin/env node
import { execFileSync } from "node:child_process";
import console from "node:console";
import { existsSync } from "node:fs";
import { dirname, resolve } from "node:path";
import process from "node:process";
import { fileURLToPath } from "node:url";
const __dirname = dirname(fileURLToPath(import.meta.url));

View File

@ -66,7 +66,7 @@ describe("notifier-composio", () => {
afterEach(() => {
for (const [key, value] of Object.entries(originalEnv)) {
if (value !== undefined) process.env[key] = value;
else delete process.env[key];
else Reflect.deleteProperty(process.env, key);
}
});

View File

@ -1,13 +1,13 @@
import { getNotificationDataV3 } from "@aoagents/ao-core";
import type {
NotificationCICheck,
NotificationDataV3,
PluginModule,
Notifier,
OrchestratorEvent,
NotifyAction,
NotifyContext,
EventPriority,
import {
getNotificationDataV3,
type NotificationCICheck,
type NotificationDataV3,
type PluginModule,
type Notifier,
type OrchestratorEvent,
type NotifyAction,
type NotifyContext,
type EventPriority,
} from "@aoagents/ao-core";
export const manifest = {

View File

@ -7,6 +7,11 @@ import { projectSessionPath } from "@/lib/routes";
import { useMuxOptional } from "@/providers/MuxProvider";
const READ_STORAGE_KEY = "ao.dashboard.notifications.read.v1";
const TRUSTED_EXTERNAL_ORIGINS: Record<string, string> = {
"github.com": "https://github.com",
"gitlab.com": "https://gitlab.com",
"linear.app": "https://linear.app",
};
type NotificationView = "all" | "unread";
@ -59,29 +64,16 @@ function getSubjectPR(notification: DashboardNotificationRecord): Record<string,
return subject ? recordField(subject, "pr") : null;
}
function sanitizeExternalUrl(value: string | null): string | null {
if (!value) return null;
try {
const parsed = new URL(value);
if (parsed.protocol === "http:" || parsed.protocol === "https:") {
return parsed.toString();
}
} catch {
return null;
}
return null;
}
function getPRUrl(notification: DashboardNotificationRecord): string | null {
const pr = getSubjectPR(notification);
return sanitizeExternalUrl(pr ? stringField(pr, "url") : null);
return pr ? stringField(pr, "url") : null;
}
function getReviewUrl(notification: DashboardNotificationRecord): string | null {
const data = notificationDataV3(notification);
if (!data) return null;
const review = recordField(data, "review");
return sanitizeExternalUrl(review ? stringField(review, "url") : null);
return review ? stringField(review, "url") : null;
}
function normalizeActionText(value: string): string {
@ -89,14 +81,42 @@ function normalizeActionText(value: string): string {
}
function canonicalUrl(value: string | null | undefined): string | null {
if (!value || value.trim().length === 0) return null;
const safeHref = safeExternalHref(value);
if (!safeHref) return null;
try {
const url = new URL(value);
const url = new URL(safeHref);
url.hash = "";
const pathname = url.pathname.replace(/\/+$/, "");
return `${url.origin}${pathname}${url.search}`;
} catch {
return value.trim().replace(/\/+$/, "");
return null;
}
}
function safeExternalHref(value: string | null | undefined): string | null {
if (!value || value.trim().length === 0) return null;
try {
const url = new URL(value.trim());
if (url.protocol !== "https:") return null;
const origin = TRUSTED_EXTERNAL_ORIGINS[url.hostname.toLowerCase()];
if (!origin) return null;
const safePath = url.pathname.split("/").map(encodePathSegment).join("/");
const safeSearch = new URLSearchParams(url.searchParams).toString();
return `${origin}${safePath}${safeSearch ? `?${safeSearch}` : ""}`;
} catch {
return null;
}
}
function encodePathSegment(segment: string): string {
try {
return encodeURIComponent(decodeURIComponent(segment));
} catch {
return encodeURIComponent(segment);
}
}
@ -204,17 +224,15 @@ function NotificationItem({
}) {
const { event } = notification;
const sessionHref = projectSessionPath(event.projectId, event.sessionId);
const prUrl = getPRUrl(notification);
const reviewUrl = getReviewUrl(notification);
const prUrl = safeExternalHref(getPRUrl(notification));
const reviewUrl = safeExternalHref(getReviewUrl(notification));
const escalationCause = getEscalationCause(notification);
const successLabel = successNotificationLabel(notification);
const label = successLabel ?? event.priority;
const urlActions = (notification.actions ?? [])
.map((action) => {
const sanitizedUrl = sanitizeExternalUrl(
typeof action.url === "string" ? action.url.trim() : null,
);
return sanitizedUrl ? { label: action.label, url: sanitizedUrl } : null;
const safeUrl = safeExternalHref(action.url);
return safeUrl ? { label: action.label, url: safeUrl } : null;
})
.filter((action): action is { label: string; url: string } => action !== null)
.filter((action) => !shouldHideRedundantAction(action, { prUrl, reviewUrl }));

View File

@ -205,4 +205,40 @@ describe("DashboardNotificationButton", () => {
"https://github.com/acme/app/actions/runs/1",
);
});
it("does not render unsafe notification URLs", () => {
muxValue.notifications = [
{
...makeNotification("1", "Suspicious notification"),
event: {
...makeNotification("1", "Suspicious notification").event,
data: makeV3Data({
subject: {
session: { id: "worker-1", projectId: "demo" },
pr: { number: 1, url: "javascript:alert(1)" },
},
review: { url: "data:text/html,<script>alert(1)</script>" },
}),
},
actions: [
{ label: "Unsafe action", url: "javascript:alert(1)" },
{ label: "Unsafe external action", url: "https://evil.example/phish" },
{ label: "Safe action", url: "https://github.com/acme/app/actions/runs/1" },
],
},
];
render(<DashboardNotificationButton />);
fireEvent.mouseEnter(screen.getByRole("button", { name: "Notifications" }));
expect(screen.queryByRole("link", { name: "PR" })).not.toBeInTheDocument();
expect(screen.queryByRole("link", { name: "Review" })).not.toBeInTheDocument();
expect(screen.queryByRole("link", { name: "Unsafe action" })).not.toBeInTheDocument();
expect(screen.queryByRole("link", { name: "Unsafe external action" })).not.toBeInTheDocument();
expect(screen.getByRole("link", { name: "Safe action" })).toHaveAttribute(
"href",
"https://github.com/acme/app/actions/runs/1",
);
});
});

View File

@ -37,7 +37,7 @@ You can also run the scriptable Slack setup directly:
```bash
export COMPOSIO_API_KEY=ak_...
ao setup composio-slack --user-id ao-agent --channel "#agents" --non-interactive
ao setup composio-slack --user-id aoagent --channel "#agents" --non-interactive
```
All Composio setup commands accept `--routing-preset urgent-only`,
@ -99,7 +99,7 @@ notifiers:
composio:
plugin: composio
defaultApp: slack # slack | discord | gmail
userId: ao-agent # Composio user id
userId: aoagent # Composio user id
connectedAccountId: ca_... # preferred when available
channelName: "#agents" # Slack
emailTo: alerts@example.com # required when defaultApp=gmail
@ -110,27 +110,27 @@ notifiers:
defaultApp: discord
mode: webhook
webhookUrl: https://discord.com/api/webhooks/...
userId: ao-agent
userId: aoagent
composio-discord-bot:
plugin: composio
defaultApp: discord
mode: bot
channelId: "1234567890"
userId: ao-agent
userId: aoagent
connectedAccountId: ca_...
composio-mail:
plugin: composio
defaultApp: gmail
emailTo: alerts@example.com
userId: ao-agent
userId: aoagent
connectedAccountId: ca_...
composio-slack:
plugin: composio
defaultApp: slack
userId: ao-agent
userId: aoagent
connectedAccountId: ca_...
channelName: "#agents"
```
@ -141,7 +141,7 @@ notifiers:
| -------------------- | --------------- | ---------------------- | -------------------------------------------------------------------- |
| `defaultApp` | ✓ | `slack` | Which Composio app to target: `slack`, `discord`, or `gmail` |
| `mode` | Discord only | auto | `webhook` uses Discord webhooks; `bot` uses bot channel messages |
| `userId` | optional | `ao-agent` | Composio user id for connected-account lookup |
| `userId` | optional | `aoagent` | Composio user id for connected-account lookup |
| `entityId` | optional | — | Backward-compatible alias for `userId` |
| `authConfigId` | setup only | — | Existing Composio Gmail auth config used by `--connect` |
| `connectedAccountId` | Slack/Gmail/bot | — | Specific connected account to use; not used for Discord webhook mode |