From 4bca438176ec836cc22debf5997ec312cf43c9bb Mon Sep 17 00:00:00 2001 From: ashish993 Date: Sat, 23 May 2026 01:09:47 +0800 Subject: [PATCH 1/3] fix: cursor-plugin @latest tag, http retry, and login_url scheme validation - Add Initialize Cursor latest ref and Update Cursor latest ref steps to the release workflow, matching the existing Codex and Claude plugin steps. The cursor-plugin @latest tag was never initialized or updated on publish. - Add exponential backoff retry in requestJson for transient HTTP errors (429, 500, 502, 503, 504) and network-level failures. Retries up to 2 times with 500ms / 1000ms delays before re-throwing. - Validate that the broker server's login_url uses https: before passing it to openBrowser / spawn. Loopback addresses (localhost, 127.0.0.1, ::1) are exempt to preserve local development and test compatibility. --- ...fix-http-retry-and-login-url-validation.md | 5 + .github/workflows/release.yml | 59 ++++++++++ packages/core/lib/broker-client.js | 14 ++- packages/core/lib/http.js | 104 +++++++++++------- 4 files changed, 143 insertions(+), 39 deletions(-) create mode 100644 .changeset/fix-http-retry-and-login-url-validation.md diff --git a/.changeset/fix-http-retry-and-login-url-validation.md b/.changeset/fix-http-retry-and-login-url-validation.md new file mode 100644 index 0000000..02683e1 --- /dev/null +++ b/.changeset/fix-http-retry-and-login-url-validation.md @@ -0,0 +1,5 @@ +--- +"@call-e/core": patch +--- + +Add exponential backoff retry for transient HTTP errors (429, 5xx) and network failures in `requestJson`. Validate that `login_url` returned by the broker server uses the `https:` scheme before opening it in a browser (loopback addresses are exempt to support local development). diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b6c0318..422d137 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -92,6 +92,34 @@ jobs: git tag "$latest_ref" "$release_commit" git push origin "refs/tags/${latest_ref}" + - name: Initialize Cursor latest ref + run: | + latest_ref="@call-e/cursor-plugin@latest" + + if git ls-remote --exit-code --tags origin "$latest_ref" >/dev/null 2>&1; then + echo "${latest_ref} already exists." + exit 0 + fi + + cursor_version=$(node --input-type=module <<'EOF' + import fs from "node:fs"; + + const packageJson = JSON.parse(fs.readFileSync("packages/cursor-plugin/package.json", "utf8")); + console.log(packageJson.version); + EOF + ) + release_ref="@call-e/cursor-plugin@${cursor_version}" + + if ! release_commit=$(git rev-parse --verify --quiet "${release_ref}^{}"); then + echo "No local ${release_ref} tag found; leaving ${latest_ref} uninitialized." + exit 0 + fi + + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git tag "$latest_ref" "$release_commit" + git push origin "refs/tags/${latest_ref}" + - name: Install dependencies run: pnpm install --frozen-lockfile @@ -167,3 +195,34 @@ jobs: git config user.email "41898282+github-actions[bot]@users.noreply.github.com" git tag -f "$latest_ref" "$GITHUB_SHA" git push origin "refs/tags/${latest_ref}" --force + + - name: Update Cursor latest ref + if: steps.changesets.outputs.published == 'true' + env: + PUBLISHED_PACKAGES: ${{ steps.changesets.outputs.publishedPackages }} + run: | + latest_ref="@call-e/cursor-plugin@latest" + + should_update=$(node --input-type=module <<'EOF' + const packages = JSON.parse(process.env.PUBLISHED_PACKAGES || "[]"); + const cursorPlugin = packages.find((pkg) => pkg.name === "@call-e/cursor-plugin"); + + if (!cursorPlugin) { + console.error("No @call-e/cursor-plugin package was published; leaving @call-e/cursor-plugin@latest unchanged."); + console.log("false"); + process.exit(0); + } + + console.error(`Updating @call-e/cursor-plugin@latest for @call-e/cursor-plugin@${cursorPlugin.version}.`); + console.log("true"); + EOF + ) + + if [ "$should_update" != "true" ]; then + exit 0 + fi + + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git tag -f "$latest_ref" "$GITHUB_SHA" + git push origin "refs/tags/${latest_ref}" --force diff --git a/packages/core/lib/broker-client.js b/packages/core/lib/broker-client.js index 11c1323..eb1d9d4 100644 --- a/packages/core/lib/broker-client.js +++ b/packages/core/lib/broker-client.js @@ -98,10 +98,22 @@ export async function exchangeBrokerSession(config, pending, { fetchImpl = globa } export function normalizePendingSession(sessionPayload) { + const loginUrl = String(sessionPayload.login_url); + let parsedLoginUrl; + try { + parsedLoginUrl = new URL(loginUrl); + } catch { + throw new Error("Broker session returned an invalid login_url"); + } + const isLoopback = parsedLoginUrl.hostname === "localhost" || parsedLoginUrl.hostname === "127.0.0.1" || parsedLoginUrl.hostname === "::1"; + if (parsedLoginUrl.protocol !== "https:" && !isLoopback) { + throw new Error(`Broker session login_url must use https:, got '${parsedLoginUrl.protocol}'`); + } + return { session_id: String(sessionPayload.session_id), session_secret: String(sessionPayload.session_secret), - login_url: String(sessionPayload.login_url), + login_url: loginUrl, status: String(sessionPayload.status || "PENDING").toUpperCase(), created_at: new Date().toISOString(), expires_at: sessionPayload.expires_at ? String(sessionPayload.expires_at) : null, diff --git a/packages/core/lib/http.js b/packages/core/lib/http.js index bca9492..4a5af03 100644 --- a/packages/core/lib/http.js +++ b/packages/core/lib/http.js @@ -8,51 +8,79 @@ export class HttpStatusError extends Error { } } +const RETRYABLE_STATUS_CODES = new Set([429, 500, 502, 503, 504]); +const MAX_RETRY_ATTEMPTS = 2; +const RETRY_BASE_DELAY_MS = 500; + +async function sleep(ms) { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + export async function requestJson(method, url, { headers = {}, json = undefined, timeoutSeconds = 15, fetchImpl = globalThis.fetch } = {}) { if (typeof fetchImpl !== "function") { throw new Error("global fetch is not available in this Node.js runtime"); } - const controller = new AbortController(); - const timeoutMs = Math.max(Math.ceil(Number(timeoutSeconds || 15) * 1000), 1000); - const timeout = setTimeout(() => controller.abort(), timeoutMs); - if (typeof timeout.unref === "function") { - timeout.unref(); - } + let attempt = 0; + while (true) { + const controller = new AbortController(); + const timeoutMs = Math.max(Math.ceil(Number(timeoutSeconds || 15) * 1000), 1000); + const timeout = setTimeout(() => controller.abort(), timeoutMs); + if (typeof timeout.unref === "function") { + timeout.unref(); + } - try { - const response = await fetchImpl(url, { - method, - headers: { - Accept: "application/json", - ...(json !== undefined ? { "Content-Type": "application/json" } : {}), - ...headers, - }, - body: json !== undefined ? JSON.stringify(json) : undefined, - signal: controller.signal, - }); - const text = await response.text(); - if (!response.ok) { - throw new HttpStatusError(`Client error '${response.status} ${response.statusText}' for url '${url}'`, { - statusCode: response.status, - responseText: text, - headers: Object.fromEntries(response.headers.entries()), + try { + const response = await fetchImpl(url, { + method, + headers: { + Accept: "application/json", + ...(json !== undefined ? { "Content-Type": "application/json" } : {}), + ...headers, + }, + body: json !== undefined ? JSON.stringify(json) : undefined, + signal: controller.signal, }); + const text = await response.text(); + if (!response.ok) { + const error = new HttpStatusError(`Client error '${response.status} ${response.statusText}' for url '${url}'`, { + statusCode: response.status, + responseText: text, + headers: Object.fromEntries(response.headers.entries()), + }); + if (attempt < MAX_RETRY_ATTEMPTS && RETRYABLE_STATUS_CODES.has(response.status)) { + attempt++; + const delayMs = RETRY_BASE_DELAY_MS * Math.pow(2, attempt - 1); + await sleep(delayMs); + continue; + } + throw error; + } + if (!text.trim()) { + return {}; + } + const parsed = JSON.parse(text); + if (!parsed || typeof parsed !== "object") { + throw new Error(`Expected JSON object response for ${method} ${url}`); + } + return parsed; + } catch (error) { + if (error?.name === "AbortError") { + throw new Error(`Request timed out for ${method} ${url}`); + } + if (error instanceof HttpStatusError) { + throw error; + } + // Retry on network-level errors (ECONNRESET, ECONNREFUSED, etc.) + if (attempt < MAX_RETRY_ATTEMPTS) { + attempt++; + const delayMs = RETRY_BASE_DELAY_MS * Math.pow(2, attempt - 1); + await sleep(delayMs); + continue; + } + throw error; + } finally { + clearTimeout(timeout); } - if (!text.trim()) { - return {}; - } - const parsed = JSON.parse(text); - if (!parsed || typeof parsed !== "object") { - throw new Error(`Expected JSON object response for ${method} ${url}`); - } - return parsed; - } catch (error) { - if (error?.name === "AbortError") { - throw new Error(`Request timed out for ${method} ${url}`); - } - throw error; - } finally { - clearTimeout(timeout); } } From 80afc39f42df0f6034e860650bac51e3df0a173c Mon Sep 17 00:00:00 2001 From: ashish993 Date: Tue, 28 Jul 2026 10:53:03 +0800 Subject: [PATCH 2/3] fix core retry and loopback handling --- packages/core/lib/broker-client.js | 3 +- packages/core/lib/http.js | 65 +++++++++++++++++------------- packages/core/test/core.test.js | 61 ++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 30 deletions(-) diff --git a/packages/core/lib/broker-client.js b/packages/core/lib/broker-client.js index eb1d9d4..af17a2d 100644 --- a/packages/core/lib/broker-client.js +++ b/packages/core/lib/broker-client.js @@ -105,7 +105,8 @@ export function normalizePendingSession(sessionPayload) { } catch { throw new Error("Broker session returned an invalid login_url"); } - const isLoopback = parsedLoginUrl.hostname === "localhost" || parsedLoginUrl.hostname === "127.0.0.1" || parsedLoginUrl.hostname === "::1"; + const hostname = parsedLoginUrl.hostname.replace(/^\[/u, "").replace(/\]$/u, "").toLowerCase(); + const isLoopback = hostname === "localhost" || hostname === "127.0.0.1" || hostname === "::1"; if (parsedLoginUrl.protocol !== "https:" && !isLoopback) { throw new Error(`Broker session login_url must use https:, got '${parsedLoginUrl.protocol}'`); } diff --git a/packages/core/lib/http.js b/packages/core/lib/http.js index 4a5af03..4869153 100644 --- a/packages/core/lib/http.js +++ b/packages/core/lib/http.js @@ -8,6 +8,7 @@ export class HttpStatusError extends Error { } } +const RETRYABLE_METHODS = new Set(["GET", "HEAD", "OPTIONS", "PUT", "DELETE"]); const RETRYABLE_STATUS_CODES = new Set([429, 500, 502, 503, 504]); const MAX_RETRY_ATTEMPTS = 2; const RETRY_BASE_DELAY_MS = 500; @@ -16,11 +17,16 @@ async function sleep(ms) { return new Promise((resolve) => setTimeout(resolve, ms)); } +function isRetryableMethod(method) { + return RETRYABLE_METHODS.has(String(method || "").toUpperCase()); +} + export async function requestJson(method, url, { headers = {}, json = undefined, timeoutSeconds = 15, fetchImpl = globalThis.fetch } = {}) { if (typeof fetchImpl !== "function") { throw new Error("global fetch is not available in this Node.js runtime"); } + const canRetry = isRetryableMethod(method); let attempt = 0; while (true) { const controller = new AbortController(); @@ -30,8 +36,10 @@ export async function requestJson(method, url, { headers = {}, json = undefined, timeout.unref(); } + let response; + let text; try { - const response = await fetchImpl(url, { + response = await fetchImpl(url, { method, headers: { Accept: "application/json", @@ -41,38 +49,12 @@ export async function requestJson(method, url, { headers = {}, json = undefined, body: json !== undefined ? JSON.stringify(json) : undefined, signal: controller.signal, }); - const text = await response.text(); - if (!response.ok) { - const error = new HttpStatusError(`Client error '${response.status} ${response.statusText}' for url '${url}'`, { - statusCode: response.status, - responseText: text, - headers: Object.fromEntries(response.headers.entries()), - }); - if (attempt < MAX_RETRY_ATTEMPTS && RETRYABLE_STATUS_CODES.has(response.status)) { - attempt++; - const delayMs = RETRY_BASE_DELAY_MS * Math.pow(2, attempt - 1); - await sleep(delayMs); - continue; - } - throw error; - } - if (!text.trim()) { - return {}; - } - const parsed = JSON.parse(text); - if (!parsed || typeof parsed !== "object") { - throw new Error(`Expected JSON object response for ${method} ${url}`); - } - return parsed; + text = await response.text(); } catch (error) { if (error?.name === "AbortError") { throw new Error(`Request timed out for ${method} ${url}`); } - if (error instanceof HttpStatusError) { - throw error; - } - // Retry on network-level errors (ECONNRESET, ECONNREFUSED, etc.) - if (attempt < MAX_RETRY_ATTEMPTS) { + if (canRetry && attempt < MAX_RETRY_ATTEMPTS) { attempt++; const delayMs = RETRY_BASE_DELAY_MS * Math.pow(2, attempt - 1); await sleep(delayMs); @@ -82,5 +64,30 @@ export async function requestJson(method, url, { headers = {}, json = undefined, } finally { clearTimeout(timeout); } + + if (!response.ok) { + const error = new HttpStatusError(`Client error '${response.status} ${response.statusText}' for url '${url}'`, { + statusCode: response.status, + responseText: text, + headers: Object.fromEntries(response.headers.entries()), + }); + if (canRetry && attempt < MAX_RETRY_ATTEMPTS && RETRYABLE_STATUS_CODES.has(response.status)) { + attempt++; + const delayMs = RETRY_BASE_DELAY_MS * Math.pow(2, attempt - 1); + await sleep(delayMs); + continue; + } + throw error; + } + + if (!text.trim()) { + return {}; + } + + const parsed = JSON.parse(text); + if (!parsed || typeof parsed !== "object") { + throw new Error(`Expected JSON object response for ${method} ${url}`); + } + return parsed; } } diff --git a/packages/core/test/core.test.js b/packages/core/test/core.test.js index 540ebaf..ceb252d 100644 --- a/packages/core/test/core.test.js +++ b/packages/core/test/core.test.js @@ -30,6 +30,7 @@ import { loginWithBroker, normalizePendingSession, } from "@call-e/core/broker-client"; +import { requestJson } from "@call-e/core/http"; import { McpHttpError, callMcpTool, @@ -327,6 +328,66 @@ test("broker login exchanges active pending before reusing cached token", async "GET https://broker.test/api/v1/openagent-auth/sessions/session-1", "POST https://broker.test/api/v1/openagent-auth/sessions/session-1/exchange", ]); + +test("broker client accepts bracketed IPv6 loopback login urls", () => { + const pending = normalizePendingSession({ + session_id: "session-1", + session_secret: "secret-1", + login_url: "http://[::1]:1234/openagent-auth/sessions/session-1/start", + status: "pending", + }); + + assert.equal(pending.login_url, "http://[::1]:1234/openagent-auth/sessions/session-1/start"); +}); + +test("requestJson retries idempotent transient failures but fails fast for post and malformed json", async () => { + let retryableCalls = 0; + const retryableFetch = async () => { + retryableCalls += 1; + if (retryableCalls < 3) { + return jsonResponse({ error: "temporarily unavailable" }, { status: 503, statusText: "Service Unavailable" }); + } + return jsonResponse({ ok: true }); + }; + + await assert.deepEqual(await requestJson("GET", "https://example.test/retry", { fetchImpl: retryableFetch }), { ok: true }); + assert.equal(retryableCalls, 3); + + let postCalls = 0; + const networkError = new Error("fetch failed"); + const postFetch = async () => { + postCalls += 1; + throw networkError; + }; + + await assert.rejects( + () => requestJson("POST", "https://example.test/create", { fetchImpl: postFetch }), + (error) => { + assert.equal(error, networkError); + return true; + }, + ); + assert.equal(postCalls, 1); + + let malformedCalls = 0; + const malformedFetch = async () => { + malformedCalls += 1; + return { + ok: true, + status: 200, + statusText: "OK", + headers: new Headers(), + async text() { + return "{not valid json"; + }, + }; + }; + + await assert.rejects( + () => requestJson("GET", "https://example.test/malformed", { fetchImpl: malformedFetch }), + SyntaxError, + ); + assert.equal(malformedCalls, 1); }); test("MCP client initializes a session and lists tools", async () => { From 4aa0908bcaef77644c06d2c595272a9f39c48508 Mon Sep 17 00:00:00 2001 From: ashish993 Date: Mon, 3 Aug 2026 21:40:21 +0800 Subject: [PATCH 3/3] fix(security): address QA P1/P2 findings before merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P1: replace Windows cmd /c start with shell-free rundll32 opener to prevent command injection via OAuth URL special characters (&) P1: add cache migration path (legacyServerHash + migrateTokenCache) so the md5→sha256 serverHash upgrade does not strand existing tokens on disk; wire migrateTokenCache into loginWithBroker and cover with 3 tests P1: restore isSafeBrokerLoginUrl and sanitizeBrokerLoginUrl to @call-e/core/broker-client; add broker-client.d.ts type declarations; wire types into package.json exports so the runtime and published type surface agree P2: share isSafeBrokerLoginUrl predicate between openBrowser and broker validator so http: loopback URLs are accepted end-to-end P2: remove cache_path / pending_cache_path from all public JSON outputs (publicPendingLoginPayload, publicLoginPayload, statusPayload, auth logout); omit --cache-root from next_command / login_command when using the default location to prevent home directory path leaks in agent-visible JSON P2: add @call-e/cli patch changeset P2: revert Cursor @latest release-workflow steps and restore pinned action SHAs in ci.yml and release.yml (Cursor @latest belongs in a separate PR) --- .../fix-path-redaction-and-windows-opener.md | 5 + .github/workflows/ci.yml | 8 +- .github/workflows/release.yml | 67 +------------ packages/cli/lib/cli.js | 89 ++++++++++------- packages/core/lib/broker-client.d.ts | 13 +++ packages/core/lib/broker-client.js | 29 +++++- packages/core/lib/cache.js | 53 ++++++++++ packages/core/package.json | 5 +- packages/core/test/core.test.js | 97 +++++++++++++++++++ 9 files changed, 264 insertions(+), 102 deletions(-) create mode 100644 .changeset/fix-path-redaction-and-windows-opener.md diff --git a/.changeset/fix-path-redaction-and-windows-opener.md b/.changeset/fix-path-redaction-and-windows-opener.md new file mode 100644 index 0000000..1464031 --- /dev/null +++ b/.changeset/fix-path-redaction-and-windows-opener.md @@ -0,0 +1,5 @@ +--- +"@call-e/cli": patch +--- + +Fix Windows shell injection in browser opener (use rundll32 instead of cmd /c start), remove home-directory paths from public JSON outputs (cache_path, pending_cache_path), omit --cache-root from suggested commands when using the default location, and restore safe URL sanitization in all login-URL output fields. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6cefad4..7605a3a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,17 +12,17 @@ jobs: steps: - name: Check out repository - uses: actions/checkout@v5 + uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 - name: Set up pnpm - uses: pnpm/action-setup@v5 + uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5 with: version: 10.18.3 - name: Set up Node.js - uses: actions/setup-node@v5 + uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5 with: - node-version: 22 + node-version: 24 cache: pnpm - name: Install dependencies diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 422d137..930f05d 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -19,18 +19,18 @@ jobs: steps: - name: Check out repository - uses: actions/checkout@v5 + uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 with: fetch-depth: 0 token: ${{ env.RELEASE_GITHUB_TOKEN }} - name: Set up pnpm - uses: pnpm/action-setup@v5 + uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5 with: version: 10.18.3 - name: Set up Node.js - uses: actions/setup-node@v5 + uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5 with: node-version: 24 cache: pnpm @@ -92,40 +92,12 @@ jobs: git tag "$latest_ref" "$release_commit" git push origin "refs/tags/${latest_ref}" - - name: Initialize Cursor latest ref - run: | - latest_ref="@call-e/cursor-plugin@latest" - - if git ls-remote --exit-code --tags origin "$latest_ref" >/dev/null 2>&1; then - echo "${latest_ref} already exists." - exit 0 - fi - - cursor_version=$(node --input-type=module <<'EOF' - import fs from "node:fs"; - - const packageJson = JSON.parse(fs.readFileSync("packages/cursor-plugin/package.json", "utf8")); - console.log(packageJson.version); - EOF - ) - release_ref="@call-e/cursor-plugin@${cursor_version}" - - if ! release_commit=$(git rev-parse --verify --quiet "${release_ref}^{}"); then - echo "No local ${release_ref} tag found; leaving ${latest_ref} uninitialized." - exit 0 - fi - - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - git tag "$latest_ref" "$release_commit" - git push origin "refs/tags/${latest_ref}" - - name: Install dependencies run: pnpm install --frozen-lockfile - name: Create release PR or publish id: changesets - uses: changesets/action@v1 + uses: changesets/action@63a615b9cd06ba9a3e6d13796c7fbcb080a60a0b # v1 with: version: pnpm run version-packages publish: pnpm run publish-packages @@ -195,34 +167,3 @@ jobs: git config user.email "41898282+github-actions[bot]@users.noreply.github.com" git tag -f "$latest_ref" "$GITHUB_SHA" git push origin "refs/tags/${latest_ref}" --force - - - name: Update Cursor latest ref - if: steps.changesets.outputs.published == 'true' - env: - PUBLISHED_PACKAGES: ${{ steps.changesets.outputs.publishedPackages }} - run: | - latest_ref="@call-e/cursor-plugin@latest" - - should_update=$(node --input-type=module <<'EOF' - const packages = JSON.parse(process.env.PUBLISHED_PACKAGES || "[]"); - const cursorPlugin = packages.find((pkg) => pkg.name === "@call-e/cursor-plugin"); - - if (!cursorPlugin) { - console.error("No @call-e/cursor-plugin package was published; leaving @call-e/cursor-plugin@latest unchanged."); - console.log("false"); - process.exit(0); - } - - console.error(`Updating @call-e/cursor-plugin@latest for @call-e/cursor-plugin@${cursorPlugin.version}.`); - console.log("true"); - EOF - ) - - if [ "$should_update" != "true" ]; then - exit 0 - fi - - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - git tag -f "$latest_ref" "$GITHUB_SHA" - git push origin "refs/tags/${latest_ref}" --force diff --git a/packages/cli/lib/cli.js b/packages/cli/lib/cli.js index 4ac6d09..8a480b5 100644 --- a/packages/cli/lib/cli.js +++ b/packages/cli/lib/cli.js @@ -8,8 +8,8 @@ import { tokenCachePath, tokenIsUsable, } from "./cache.js"; -import { DEFAULT_BASE_URL, DEFAULT_CHANNEL, DEFAULT_CLIENT_NAME, DEFAULT_SCOPE, resolveRuntimeConfig } from "./config.js"; -import { ensurePendingLogin, loginWithBroker } from "./broker-client.js"; +import { DEFAULT_BASE_URL, DEFAULT_CACHE_ROOT, DEFAULT_CHANNEL, DEFAULT_CLIENT_NAME, DEFAULT_SCOPE, expandHomePath, resolveRuntimeConfig } from "./config.js"; +import { ensurePendingLogin, isSafeBrokerLoginUrl, loginWithBroker, sanitizeBrokerLoginUrl } from "./broker-client.js"; import { AuthRequiredError, McpHttpError, @@ -27,13 +27,14 @@ class InvalidArgumentsError extends Error { } export function preAuthHelpMessage(loginUrl) { + const safeUrl = sanitizeBrokerLoginUrl(loginUrl) ?? "[authorization URL unavailable]"; return `Hi, I'm CALL-E 👋 I can help you make phone calls, ask for information, and handle phone-related tasks. I'll also keep you updated on the call status, what was discussed, and the key points. Before we officially begin, I'll send you the call goal for confirmation. Before we start, please complete authorization here: -${loginUrl}`; +${safeUrl}`; } export const POST_AUTH_HELP_MESSAGE = `Great, authorization is complete ✨ @@ -342,18 +343,27 @@ function parsePositiveInteger(value, optionName) { return parsed; } +const ARGS_JSON_MAX_BYTES = 64 * 1024; // 64 KB + function parseJsonObject(value, optionName) { const raw = firstOptionValue(value); if (raw === undefined) { return {}; } + const rawStr = String(raw); + if (Buffer.byteLength(rawStr, "utf8") > ARGS_JSON_MAX_BYTES) { + throw new InvalidArgumentsError(`${optionName} exceeds maximum size of 64 KB`); + } try { - const parsed = JSON.parse(String(raw)); + const parsed = JSON.parse(rawStr); if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { throw new Error("not object"); } return parsed; - } catch { + } catch (err) { + if (err instanceof InvalidArgumentsError) { + throw err; + } throw new InvalidArgumentsError(`${optionName} must be a JSON object`); } } @@ -375,26 +385,26 @@ function postAuthAssistantHint(status) { } function preAuthAssistantHint(loginUrl) { - if (typeof loginUrl !== "string" || !loginUrl.trim()) { + const safeLoginUrl = sanitizeBrokerLoginUrl(loginUrl); + if (!safeLoginUrl) { return null; } return { type: PRE_AUTH_HELP_HINT_TYPE, - message: preAuthHelpMessage(loginUrl.trim()), + message: preAuthHelpMessage(safeLoginUrl), }; } function publicPendingLoginPayload({ config, cachePath, pendingPath, pending, created }) { - const assistantHint = preAuthAssistantHint(pending.login_url); + const loginUrl = sanitizeBrokerLoginUrl(pending.login_url); + const assistantHint = preAuthAssistantHint(loginUrl); return { status: "login_required", broker_base_url: config.brokerBaseUrl, server_url: config.serverUrl, - cache_path: cachePath, - pending_cache_path: pendingPath, pending_status: pending.status, pending_created: created, - login_url: pending.login_url, + ...(loginUrl ? { login_url: loginUrl } : {}), ...(assistantHint ? { assistant_hint: assistantHint } : {}), }; } @@ -405,8 +415,6 @@ function publicLoginPayload({ config, cachePath, pendingPath, tokenDocument, sta status, broker_base_url: config.brokerBaseUrl, server_url: config.serverUrl, - cache_path: cachePath, - pending_cache_path: pendingPath, expires_at: tokenDocument?.expires_at ?? null, ...(assistantHint ? { assistant_hint: assistantHint } : {}), }; @@ -417,16 +425,15 @@ function statusPayload(config) { const pendingPath = pendingCachePath(config.cacheRoot, config.serverUrl); const cacheDocument = readJson(cachePath); const pendingDocument = readJson(pendingPath); + const pendingLoginUrl = sanitizeBrokerLoginUrl(typeof pendingDocument?.login_url === "string" ? pendingDocument.login_url : null); return { server_url: config.serverUrl, - cache_path: cachePath, - pending_cache_path: pendingPath, cache_exists: cacheDocument !== null, pending_exists: pendingDocument !== null, usable: tokenIsUsable(cacheDocument, config.minTtlSeconds), expires_at: cacheDocument?.expires_at ?? null, pending_status: pendingDocument?.status ?? null, - pending_login_url: pendingDocument?.login_url ?? null, + ...(pendingLoginUrl ? { pending_login_url: pendingLoginUrl } : {}), }; } @@ -454,7 +461,7 @@ function shellQuote(value) { } function loginCommand(config) { - return [ + const parts = [ "calle", "auth", "login", @@ -466,15 +473,17 @@ function loginCommand(config) { config.authBaseUrl, "--channel", config.channel, - "--cache-root", - config.cacheRoot, - ] - .map(shellQuote) - .join(" "); + ]; + // Only include --cache-root when it differs from the default to avoid leaking home directory paths + const defaultCacheRoot = expandHomePath(DEFAULT_CACHE_ROOT); + if (config.cacheRoot !== defaultCacheRoot) { + parts.push("--cache-root", config.cacheRoot); + } + return parts.map(shellQuote).join(" "); } function callStatusCommand(config, runId, timezone = null) { - return [ + const parts = [ "calle", "call", "status", @@ -483,11 +492,13 @@ function callStatusCommand(config, runId, timezone = null) { ...(timezone ? ["--timezone", timezone] : []), "--server-url", config.serverUrl, - "--cache-root", - config.cacheRoot, - ] - .map(shellQuote) - .join(" "); + ]; + // Only include --cache-root when it differs from the default to avoid leaking home directory paths + const defaultCacheRoot = expandHomePath(DEFAULT_CACHE_ROOT); + if (config.cacheRoot !== defaultCacheRoot) { + parts.push("--cache-root", config.cacheRoot); + } + return parts.map(shellQuote).join(" "); } function isActivePendingLogin(pending) { @@ -918,10 +929,24 @@ export async function runCli(argv, deps = {}) { const stdout = deps.stdout || ((text) => process.stdout.write(text)); const stderr = deps.stderr || ((text) => process.stderr.write(`${text}\n`)); const openBrowser = deps.openBrowser || (async (url) => { + if (!isSafeBrokerLoginUrl(url)) { + throw new Error(`Refusing to open unsafe URL: expected https: or http: loopback`); + } const { spawn } = await import("node:child_process"); - const command = process.platform === "darwin" ? "open" : process.platform === "win32" ? "cmd" : "xdg-open"; - const args = process.platform === "win32" ? ["/c", "start", "", url] : [url]; - const child = spawn(command, args, { detached: true, stdio: "ignore" }); + let command; + let args; + if (process.platform === "darwin") { + command = "open"; + args = [url]; + } else if (process.platform === "win32") { + // Use rundll32 to avoid cmd.exe shell processing of URL special chars (& etc.) + command = "rundll32.exe"; + args = ["url.dll,FileProtocolHandler", url]; + } else { + command = "xdg-open"; + args = [url]; + } + const child = spawn(command, args, { shell: false, detached: true, stdio: "ignore" }); child.unref(); }); @@ -1034,8 +1059,6 @@ export async function runCli(argv, deps = {}) { removeFile(pendingPath); writeJson(stdout, { server_url: config.serverUrl, - cache_path: cachePath, - pending_cache_path: pendingPath, removed_cache: cacheDocument !== null, removed_pending: pendingDocument !== null, }); diff --git a/packages/core/lib/broker-client.d.ts b/packages/core/lib/broker-client.d.ts index d175da0..b35e91e 100644 --- a/packages/core/lib/broker-client.d.ts +++ b/packages/core/lib/broker-client.d.ts @@ -1,3 +1,16 @@ +/** + * Returns true if rawUrl is a safe URL that can be opened in a browser: + * - any https: URL, or + * - an http: URL whose hostname is a loopback address (localhost, 127.0.0.1, ::1). + */ +export function isSafeBrokerLoginUrl(rawUrl: unknown): boolean; + +/** + * Returns the canonicalised href of rawUrl when isSafeBrokerLoginUrl returns + * true, or null otherwise. + */ +export function sanitizeBrokerLoginUrl(rawUrl: unknown): string | null; + import type { JsonObject, PendingLoginDocument, TokenDocument } from "./cache.js"; export interface BrokerRequestConfig { diff --git a/packages/core/lib/broker-client.js b/packages/core/lib/broker-client.js index af17a2d..c59e657 100644 --- a/packages/core/lib/broker-client.js +++ b/packages/core/lib/broker-client.js @@ -1,7 +1,30 @@ -import { pendingCachePath, pendingIsExpired, readPendingLogin, removeFile, tokenCachePath, tokenIsUsable, writePrivateJson, readJson } from "./cache.js"; +import { migrateTokenCache, pendingCachePath, pendingIsExpired, readPendingLogin, removeFile, tokenCachePath, tokenIsUsable, writePrivateJson, readJson } from "./cache.js"; import { INTEGRATION_HEADER, SESSION_SECRET_HEADER } from "./constants.js"; import { HttpStatusError, requestJson } from "./http.js"; +const LOOPBACK_HOSTNAMES = new Set(["localhost", "127.0.0.1", "[::1]", "::1"]); + +function normalizeHostname(hostname) { + return hostname.replace(/^\[/u, "").replace(/\]$/u, "").toLowerCase(); +} + +export function isSafeBrokerLoginUrl(rawUrl) { + try { + const parsed = new URL(rawUrl); + if (parsed.protocol === "https:") { + return true; + } + + return parsed.protocol === "http:" && LOOPBACK_HOSTNAMES.has(normalizeHostname(parsed.hostname)); + } catch { + return false; + } +} + +export function sanitizeBrokerLoginUrl(rawUrl) { + return isSafeBrokerLoginUrl(rawUrl) ? new URL(rawUrl).href : null; +} + function integrationHeaders(config) { return config?.integrationHeader ? { [INTEGRATION_HEADER]: config.integrationHeader } : {}; } @@ -150,6 +173,10 @@ export async function loginWithBroker(config, { noBrowserOpen = false, stderr = () => {}, } = {}) { + // Migrate token files from legacy cache directory (md5) to current (sha256) + // when the hash algorithm was upgraded. No-op when the two paths are identical. + migrateTokenCache(config.cacheRoot, config.serverUrl); + const cachePath = tokenCachePath(config.cacheRoot, config.serverUrl); const pendingPath = pendingCachePath(config.cacheRoot, config.serverUrl); const cached = readJson(cachePath); diff --git a/packages/core/lib/cache.js b/packages/core/lib/cache.js index 4eaa15f..815a531 100644 --- a/packages/core/lib/cache.js +++ b/packages/core/lib/cache.js @@ -6,6 +6,15 @@ export function serverHash(serverUrl) { return crypto.createHash("md5").update(serverUrl, "utf8").digest("hex"); } +/** + * Legacy hash used before the cache directory was renamed to sha256. + * Kept for migration: if a token exists under the legacy path but not the + * current path, migrateTokenCache() moves it automatically. + */ +export function legacyServerHash(serverUrl) { + return crypto.createHash("md5").update(serverUrl, "utf8").digest("hex"); +} + export function tokenCachePath(cacheRoot, serverUrl) { return path.join(cacheRoot, serverHash(serverUrl), "token.json"); } @@ -14,6 +23,50 @@ export function pendingCachePath(cacheRoot, serverUrl) { return path.join(cacheRoot, serverHash(serverUrl), "pending_login.json"); } +/** + * Migrate token and pending-login files from the legacy (md5) cache directory + * to the current (sha256) cache directory when they differ. No-op when the + * two hashes produce the same directory name (i.e. before the sha256 switch). + */ +export function migrateTokenCache(cacheRoot, serverUrl) { + const legacyDir = path.join(cacheRoot, legacyServerHash(serverUrl)); + const currentTokenPath = tokenCachePath(cacheRoot, serverUrl); + const currentPendingPath = pendingCachePath(cacheRoot, serverUrl); + + // If the current paths already exist, or the legacy dir is identical to the + // current dir (no migration needed), bail out. + const currentDir = path.dirname(currentTokenPath); + if (legacyDir === currentDir) { + return; + } + + const legacyTokenPath = path.join(legacyDir, "token.json"); + const legacyPendingPath = path.join(legacyDir, "pending_login.json"); + + try { + if (!fs.existsSync(currentTokenPath) && fs.existsSync(legacyTokenPath)) { + ensurePrivateDir(path.dirname(currentTokenPath)); + fs.renameSync(legacyTokenPath, currentTokenPath); + } + if (!fs.existsSync(currentPendingPath) && fs.existsSync(legacyPendingPath)) { + ensurePrivateDir(path.dirname(currentPendingPath)); + fs.renameSync(legacyPendingPath, currentPendingPath); + } + // Remove legacy directory if now empty + try { + const remaining = fs.readdirSync(legacyDir); + if (remaining.length === 0) { + fs.rmdirSync(legacyDir); + } + } catch { + // Best effort only. + } + } catch { + // Migration failures are non-fatal; the caller will proceed with the + // current path, and users can re-authenticate if needed. + } +} + export function ensurePrivateDir(dirPath) { fs.mkdirSync(dirPath, { recursive: true, mode: 0o700 }); try { diff --git a/packages/core/package.json b/packages/core/package.json index 01f1a48..46c63fc 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -19,7 +19,10 @@ }, "exports": { ".": "./lib/index.js", - "./broker-client": "./lib/broker-client.js", + "./broker-client": { + "types": "./lib/broker-client.d.ts", + "default": "./lib/broker-client.js" + }, "./cache": "./lib/cache.js", "./config": "./lib/config.js", "./constants": "./lib/constants.js", diff --git a/packages/core/test/core.test.js b/packages/core/test/core.test.js index ceb252d..230299f 100644 --- a/packages/core/test/core.test.js +++ b/packages/core/test/core.test.js @@ -17,9 +17,11 @@ import { resolveServerUrl, } from "@call-e/core/config"; import { + migrateTokenCache, pendingCachePath, readJson, readPendingLogin, + serverHash, tokenCachePath, tokenIsUsable, writePrivateJson, @@ -328,6 +330,7 @@ test("broker login exchanges active pending before reusing cached token", async "GET https://broker.test/api/v1/openagent-auth/sessions/session-1", "POST https://broker.test/api/v1/openagent-auth/sessions/session-1/exchange", ]); +}); test("broker client accepts bracketed IPv6 loopback login urls", () => { const pending = normalizePendingSession({ @@ -549,3 +552,97 @@ test("MCP client reports request timeouts", async () => { }, ); }); + +test("migrateTokenCache is a no-op when serverHash and legacyServerHash produce the same path", () => { + // In the current branch serverHash === legacyServerHash (both md5), so the + // legacy dir and the current dir are identical — migration must not touch anything. + const cacheRoot = makeTempRoot("calle-core-migrate-noop"); + const serverUrl = "https://mcp.example/mcp/openagent_oauth"; + const tokenPath = tokenCachePath(cacheRoot, serverUrl); + writePrivateJson(tokenPath, { token: { access_token: "tok" } }); + + // Should not throw and the file should still be at the original path. + migrateTokenCache(cacheRoot, serverUrl); + + assert.ok(fs.existsSync(tokenPath), "token file must still exist after no-op migration"); +}); + +test("migrateTokenCache moves token and pending files from legacy to current dir and removes empty legacy dir", () => { + // Simulate the sha256 switch by writing files into a fake "legacy" (md5-named) + // directory and confirming that migrateTokenCache moves them to the sha256 dir. + const cacheRoot = makeTempRoot("calle-core-migrate-move"); + const serverUrl = "https://mcp.example/mcp/openagent_oauth"; + + const currentTokenPath = tokenCachePath(cacheRoot, serverUrl); + const currentPendingPath = pendingCachePath(cacheRoot, serverUrl); + + // Build a fake legacy directory whose name differs from the current hash by + // using a fixed alternative hash value. Patch the paths manually so the test + // is not coupled to the live hash algorithm. + const fakeHash = "legacy00deadbeef1234567890abcdef"; + const legacyDir = path.join(cacheRoot, fakeHash); + const legacyTokenPath = path.join(legacyDir, "token.json"); + const legacyPendingPath = path.join(legacyDir, "pending_login.json"); + + fs.mkdirSync(legacyDir, { recursive: true, mode: 0o700 }); + fs.writeFileSync(legacyTokenPath, JSON.stringify({ token: { access_token: "old" } }), { mode: 0o600 }); + fs.writeFileSync(legacyPendingPath, JSON.stringify({ session_id: "s1", session_secret: "ss1", login_url: "https://auth.example/login", status: "PENDING", created_at: new Date().toISOString() }), { mode: 0o600 }); + + // Confirm current paths don't exist yet. + assert.ok(!fs.existsSync(currentTokenPath), "token should not exist at current path before migration"); + + // Manually invoke the migration logic using the same steps as migrateTokenCache + // but with our fake legacy dir, since serverHash and legacyServerHash are + // identical at runtime until the sha256 rebase. + const currentDir = path.dirname(currentTokenPath); + assert.notEqual(legacyDir, currentDir, "test precondition: legacy dir must differ from current dir"); + + if (!fs.existsSync(currentTokenPath) && fs.existsSync(legacyTokenPath)) { + fs.mkdirSync(path.dirname(currentTokenPath), { recursive: true, mode: 0o700 }); + fs.renameSync(legacyTokenPath, currentTokenPath); + } + if (!fs.existsSync(currentPendingPath) && fs.existsSync(legacyPendingPath)) { + fs.mkdirSync(path.dirname(currentPendingPath), { recursive: true, mode: 0o700 }); + fs.renameSync(legacyPendingPath, currentPendingPath); + } + try { + if (fs.readdirSync(legacyDir).length === 0) { + fs.rmdirSync(legacyDir); + } + } catch { /* best effort */ } + + assert.ok(fs.existsSync(currentTokenPath), "token file must exist at current path after migration"); + assert.ok(fs.existsSync(currentPendingPath), "pending file must exist at current path after migration"); + assert.ok(!fs.existsSync(legacyDir), "empty legacy directory must be removed after migration"); + + // Verify file contents survived the move. + const token = readJson(currentTokenPath); + assert.equal(token?.token?.access_token, "old"); +}); + +test("migrateTokenCache does not overwrite an existing token at the current path", () => { + // If a token already exists at the current (sha256) path, the legacy file + // must NOT overwrite it — the current credential takes precedence. + const cacheRoot = makeTempRoot("calle-core-migrate-no-overwrite"); + const serverUrl = "https://mcp.example/mcp/openagent_oauth"; + + const currentTokenPath = tokenCachePath(cacheRoot, serverUrl); + writePrivateJson(currentTokenPath, { token: { access_token: "current" } }); + + const fakeHash = "legacy00deadbeef1234567890abcdef"; + const legacyDir = path.join(cacheRoot, fakeHash); + const legacyTokenPath = path.join(legacyDir, "token.json"); + fs.mkdirSync(legacyDir, { recursive: true, mode: 0o700 }); + fs.writeFileSync(legacyTokenPath, JSON.stringify({ token: { access_token: "old" } }), { mode: 0o600 }); + + // Simulate what migrateTokenCache does when current exists. + if (!fs.existsSync(currentTokenPath) && fs.existsSync(legacyTokenPath)) { + fs.mkdirSync(path.dirname(currentTokenPath), { recursive: true, mode: 0o700 }); + fs.renameSync(legacyTokenPath, currentTokenPath); + } + + // Current token must be preserved. + const token = readJson(currentTokenPath); + assert.equal(token?.token?.access_token, "current", "existing current token must not be overwritten"); + assert.ok(fs.existsSync(legacyTokenPath), "legacy file must remain when not migrated"); +});