From 797fac9231813e5a3d95731b887b974a328297f3 Mon Sep 17 00:00:00 2001 From: Alessandro Pogliaghi Date: Wed, 29 Jul 2026 15:36:44 +0100 Subject: [PATCH] fix(agents): validate the agent slug before building the ingress URL The agent slug becomes a subdomain label in the ingress URL (`.agents..posthog.com`), but it arrived from an `?agent=` approval deep link and was interpolated without any validation, so a slug that isn't a plain label could change where the resulting request goes. Validate it as a single DNS label in two places: the deep-link handler drops a malformed slug so the link falls back to the fleet Approvals inbox, and `agentIngressBaseUrl` returns null rather than interpolating one, which keeps callers on their existing "no ingress" path. --- packages/core/src/links/approval-link.test.ts | 21 ++++++++---- packages/core/src/links/approval-link.ts | 15 ++++++--- packages/shared/src/agent-slug.test.ts | 32 +++++++++++++++++++ packages/shared/src/agent-slug.ts | 7 ++++ packages/shared/src/index.ts | 1 + .../agent-applications/utils/ingress.test.ts | 21 ++++++++++++ .../agent-applications/utils/ingress.ts | 4 +-- 7 files changed, 87 insertions(+), 14 deletions(-) create mode 100644 packages/shared/src/agent-slug.test.ts create mode 100644 packages/shared/src/agent-slug.ts create mode 100644 packages/ui/src/features/agent-applications/utils/ingress.test.ts diff --git a/packages/core/src/links/approval-link.test.ts b/packages/core/src/links/approval-link.test.ts index 26b67393e7..3e40abb412 100644 --- a/packages/core/src/links/approval-link.test.ts +++ b/packages/core/src/links/approval-link.test.ts @@ -102,20 +102,27 @@ describe("ApprovalLinkService", () => { expect(listener).toHaveBeenCalledWith(expected); }); - it("carries the agent slug from the ?agent= query string", () => { + it.each<{ name: string; search: string; agent: string | null }>([ + { + name: "carries the agent slug from the ?agent= query string", + search: "agent=my-agent", + agent: "my-agent", + }, + { + name: "drops a slug that is not a single DNS label", + search: "agent=evil.com%2F", + agent: null, + }, + ])("$name", ({ search, agent }) => { const listener = vi.fn(); service.on(ApprovalLinkEvent.OpenApproval, listener); - const result = deepLinkService.trigger( - "approval", - "ar_abc123", - "agent=my-agent", - ); + const result = deepLinkService.trigger("approval", "ar_abc123", search); expect(result).toBe(true); expect(listener).toHaveBeenCalledWith({ requestId: "ar_abc123", - agent: "my-agent", + agent, }); }); diff --git a/packages/core/src/links/approval-link.ts b/packages/core/src/links/approval-link.ts index 74f7f7ffda..44f422d03e 100644 --- a/packages/core/src/links/approval-link.ts +++ b/packages/core/src/links/approval-link.ts @@ -7,7 +7,7 @@ import { type IMainWindow, MAIN_WINDOW_SERVICE, } from "@posthog/platform/main-window"; -import { TypedEventEmitter } from "@posthog/shared"; +import { isValidAgentSlug, TypedEventEmitter } from "@posthog/shared"; import { inject, injectable } from "inversify"; import type { LinkLogger } from "./identifiers"; @@ -72,10 +72,15 @@ export class ApprovalLinkService extends TypedEventEmitter { return false; } - const payload: ApprovalLinkPayload = { - requestId, - agent: searchParams.get("agent") || null, - }; + const rawAgent = searchParams.get("agent"); + const agent = isValidAgentSlug(rawAgent) ? rawAgent : null; + if (rawAgent && !agent) { + this.log.warn( + "Approval link carried a malformed agent slug; ignoring it", + ); + } + + const payload: ApprovalLinkPayload = { requestId, agent }; const hasListeners = this.listenerCount(ApprovalLinkEvent.OpenApproval) > 0; diff --git a/packages/shared/src/agent-slug.test.ts b/packages/shared/src/agent-slug.test.ts new file mode 100644 index 0000000000..0c81e36428 --- /dev/null +++ b/packages/shared/src/agent-slug.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from "vitest"; +import { isValidAgentSlug } from "./agent-slug"; + +describe("isValidAgentSlug", () => { + it.each(["a", "my-agent", "agent123", "A1-b2", "x".repeat(63)])( + "accepts the DNS label %s", + (slug) => { + expect(isValidAgentSlug(slug)).toBe(true); + }, + ); + + it.each([ + "evil.com", + "evil.com/", + "evil.com#x", + "evil.com?x", + "evil.com:9999/", + "evil.com\\x", + "user@evil.com", + "foo/../bar", + "foo bar", + "-lead", + "trail-", + "under_score", + "x".repeat(64), + "", + null, + undefined, + ])("rejects %s", (slug) => { + expect(isValidAgentSlug(slug)).toBe(false); + }); +}); diff --git a/packages/shared/src/agent-slug.ts b/packages/shared/src/agent-slug.ts new file mode 100644 index 0000000000..91e02c0304 --- /dev/null +++ b/packages/shared/src/agent-slug.ts @@ -0,0 +1,7 @@ +export const AGENT_SLUG_PATTERN = /^[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?$/i; + +export function isValidAgentSlug( + slug: string | null | undefined, +): slug is string { + return !!slug && AGENT_SLUG_PATTERN.test(slug); +} diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 6d3b6b608a..f6d15bc1ae 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -19,6 +19,7 @@ export type { AgentToolKind, } from "./agent-conversation"; export * from "./agent-runtime"; +export { AGENT_SLUG_PATTERN, isValidAgentSlug } from "./agent-slug"; export * from "./analytics-events"; export { type ArchivedTask, archivedTaskSchema } from "./archive-domain"; export { withTimeout } from "./async"; diff --git a/packages/ui/src/features/agent-applications/utils/ingress.test.ts b/packages/ui/src/features/agent-applications/utils/ingress.test.ts new file mode 100644 index 0000000000..877881cccc --- /dev/null +++ b/packages/ui/src/features/agent-applications/utils/ingress.test.ts @@ -0,0 +1,21 @@ +import type { CloudRegion } from "@posthog/shared"; +import { describe, expect, it } from "vitest"; +import { agentIngressBaseUrl } from "./ingress"; + +describe("agentIngressBaseUrl", () => { + it.each<{ region: CloudRegion; expected: string }>([ + { region: "us", expected: "https://my-agent.agents.us.posthog.com" }, + { region: "eu", expected: "https://my-agent.agents.eu.posthog.com" }, + { region: "dev", expected: "http://localhost:3030/agents/my-agent" }, + ])("builds the $region URL for a valid slug", ({ region, expected }) => { + expect(agentIngressBaseUrl("my-agent", region)).toBe(expected); + }); + + it.each(["evil.com/", ""])("refuses to interpolate the slug %s", (slug) => { + expect(agentIngressBaseUrl(slug, "us")).toBeNull(); + }); + + it("returns null without a region", () => { + expect(agentIngressBaseUrl("my-agent", null)).toBeNull(); + }); +}); diff --git a/packages/ui/src/features/agent-applications/utils/ingress.ts b/packages/ui/src/features/agent-applications/utils/ingress.ts index cf5af5c19c..f4f1a11a68 100644 --- a/packages/ui/src/features/agent-applications/utils/ingress.ts +++ b/packages/ui/src/features/agent-applications/utils/ingress.ts @@ -1,4 +1,4 @@ -import type { CloudRegion } from "@posthog/shared"; +import { type CloudRegion, isValidAgentSlug } from "@posthog/shared"; /** * Resolve the agent-ingress base URL for live (streaming) calls, derived per @@ -41,7 +41,7 @@ export function agentIngressBaseUrl( slug: string, region: CloudRegion | null, ): string | null { - if (!slug || !region) return null; + if (!isValidAgentSlug(slug) || !region) return null; switch (region) { case "us": return `https://${slug}.agents.us.posthog.com`;