security: enterprise review fixes — HTTPS enforcement, input limits, path redaction, CI alignment by ashish993 · Pull Request #49 · CALLE-AI/call-e-integrations

ashish993 · 2026-05-24T07:22:49Z

Summary

Enterprise security review fixes across packages/cli, packages/core, and .github/workflows. All changes pass pnpm check and pnpm test (27 unit + 13 E2E tests).

Changes

Critical

C-2: Remove cache_path / pending_cache_path (full home-directory paths) from all public JSON outputs (publicPendingLoginPayload, publicLoginPayload, statusPayload). statusPayload now also validates pending_login_url is HTTPS before including it.
C-3: Validate login_url from broker is HTTPS before writing to stderr or passing to openBrowser.

High

H-1: Add 64 KB max-size guard on --args-json in parseJsonObject() to prevent oversized payloads.
H-2: Cap --to-phone at 10 numbers and --goal at 2000 characters in buildPlanArguments().
H-4: Enforce HTTPS on all user-supplied base URLs (--base-url, --server-url, --broker-base-url, --auth-base-url). Loopback addresses (localhost, 127.0.0.1) are allowed with HTTP for local development/testing.

Medium

M-5: Align CI workflow to Node 24 (was 22) to match the release workflow.

Low

L-1: Truncate server-controlled error messages to 200 chars and strip newlines in McpHttpError to prevent server-injected content from reaching LLM context.
L-2: CALLE_TELEMETRY_URL misconfiguration no longer throws and blocks all CLI commands — warns to stderr and disables telemetry gracefully.
L-4: loginCommand() omits --cache-root from the hint string when it equals the default (~/.calle-mcp/cli), avoiding home-directory path leakage in assistant hints.

Testing

pnpm check   # all syntax/lint checks pass
pnpm test    # 27 unit + 13 E2E tests pass

- Pin all GitHub Actions to immutable commit SHAs to prevent supply chain attacks via mutable tag references (actions/checkout, pnpm/action-setup, actions/setup-node, changesets/action) - Validate login_url is HTTPS before passing to system browser opener to prevent file:// or other protocol exploitation from a compromised broker response - Sanitize login_url in preAuthHelpMessage to HTTPS-only before embedding in LLM agent skill prompts to prevent prompt injection from a compromised broker

- Replace MD5 with SHA-256 in serverHash() cache key (packages/core/lib/cache.js) - Validate CALLE_TELEMETRY_URL is HTTPS before use (packages/cli/lib/config.js) - Add max polling attempt cap (600) to loginWithBroker alongside time deadline (packages/core/lib/broker-client.js) - Add retry with exponential backoff for 429/502/503/504 in requestJsonRpc (packages/core/lib/mcp-client.js) - Add MCP protocol version mismatch warning to stderr on initialize (packages/core/lib/mcp-client.js + cli.js)

- C-2: remove cache_path/pending_cache_path (home dir paths) from all public JSON payloads in cli.js; statusPayload now validates pending_login_url HTTPS before including it - H-1: add 64 KB max-size guard on --args-json in parseJsonObject() - H-2: add max 10 --to-phone numbers and max 2000 char --goal limit in buildPlanArguments() - L-1: truncate server-controlled error messages to 200 chars and strip newlines in McpHttpError to prevent injection into LLM context - L-4: omit --cache-root from loginCommand() output when it equals the default (~/.calle-mcp/cli) to avoid leaking home paths in hints - M-5: align CI workflow to Node 24 (matches release workflow) - Also: allow http:// on loopback (localhost/127.0.0.1) for local dev/test; HTTPS enforcement still applies for all external URLs

Copilot

Pull request overview

Security-hardening changes across the CLI and core libraries to address enterprise review findings, primarily focusing on preventing sensitive path leakage, enforcing HTTPS for externally supplied URLs, and bounding user/server-controlled inputs to reduce injection and resource-exhaustion risk.

Changes:

Enforce HTTPS (with loopback-only HTTP exception) for CLI-supplied base/server/broker/auth URLs; validate broker-provided login URLs before printing/opening.
Add input/output hardening: caps for --args-json, --to-phone, --goal; truncate/strip newlines from server-controlled MCP error messages; redact cache paths from public JSON payloads.
Align CI to Node 24 and pin GitHub Actions to specific SHAs.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
packages/core/lib/mcp-client.js	Adds retry logic, protocol mismatch warning hook, and sanitizes remote error messages.
packages/core/lib/cache.js	Changes cache key hashing from MD5 to SHA-256.
packages/core/lib/broker-client.js	Enforces HTTPS for broker login URL before printing/opening; adds polling attempt cap.
packages/cli/lib/config.js	Adds HTTPS enforcement for user-supplied endpoint URLs; makes telemetry URL misconfig non-fatal.
packages/cli/lib/cli.js	Adds input size/length limits, redacts cache paths from public outputs, and tightens login URL handling.
.gitignore	Ignores local `run.md` notes file.
.github/workflows/release.yml	Pins actions to SHAs; uses Node 24.
.github/workflows/ci.yml	Pins actions to SHAs; updates CI Node version to 24.

Comments suppressed due to low confidence (1)

packages/cli/lib/cli.js:406

publicPendingLoginPayload still returns the broker-provided login_url verbatim in JSON output. Since login_url is server-controlled, this can re-introduce unsafe/non-HTTPS URLs into public outputs even though other paths now validate/redact. Consider only including login_url when it parses as https: (or omitting/setting null otherwise), similar to statusPayload and the stderr/openBrowser safeguards.

    pending_status: pending.status,
    pending_created: created,
    login_url: pending.login_url,
    ...(assistantHint ? { assistant_hint: assistantHint } : {}),

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+async function requestJsonRpc(fetchImpl, url, { headers, payload, timeoutMs, sleepImpl = (ms) => new Promise((r) => setTimeout(r, ms)) }) {
+  let lastError;
+  for (let attempt = 1; attempt <= MAX_RETRY_ATTEMPTS; attempt++) {
+    const controller = new AbortController();
+    const timeout = setTimeout(() => controller.abort(), timeoutMs);


+          headers: responseHeaders,
+        });
+        if (RETRYABLE_STATUS_CODES.has(response.status) && attempt < MAX_RETRY_ATTEMPTS) {
+          lastError = err;


+        const safeMessage = rawMessage
+          ? rawMessage.slice(0, 200).replace(/[\r\n]+/g, " ").trim()


+  const retryAfter = Number(retryAfterHeader);
+  if (retryAfterHeader && !Number.isNaN(retryAfter) && retryAfter > 0) {
+    return Math.min(retryAfter * 1000, 30000);
  }


 export function resolveRuntimeConfig(options = {}, env = process.env) {
+  if (options.baseUrl) {
+    requireHttpsUrl(options.baseUrl, "--base-url");
+  }
+  if (options.serverUrl) {
+    requireHttpsUrl(options.serverUrl, "--server-url");
+  }
+  if (options.brokerBaseUrl) {
+    requireHttpsUrl(options.brokerBaseUrl, "--broker-base-url");
+  }
+  if (options.authBaseUrl) {
+    requireHttpsUrl(options.authBaseUrl, "--auth-base-url");
+  }
  const baseUrl = normalizeBaseUrl(options.baseUrl || DEFAULT_BASE_URL);
  const channel = options.channel || DEFAULT_CHANNEL;
  const serverUrl = resolveServerUrl({ serverUrl: options.serverUrl, baseUrl, channel });


+  const rawStr = String(raw);
+  if (Buffer.byteLength(rawStr, "utf8") > ARGS_JSON_MAX_BYTES) {
+    throw new InvalidArgumentsError(`${optionName} exceeds maximum size of 64 KB`);
+  }


+  if (toPhones.length > 10) {
+    throw new InvalidArgumentsError("--to-phone: maximum 10 numbers per request");
+  }


+  const goal = requireStringOption(options, "goal", "--goal");
+  if (goal.length > 2000) {
+    throw new InvalidArgumentsError("--goal: maximum 2000 characters");
+  }


 export function serverHash(serverUrl) {
-  return crypto.createHash("md5").update(serverUrl, "utf8").digest("hex");
+  return crypto.createHash("sha256").update(serverUrl, "utf8").digest("hex");
 }


Ray-56

Reviewed locally. I recommend holding this PR until the following issues are addressed.

requestJsonRpc retries non-idempotent tools/call requests.

packages/core/lib/mcp-client.js now retries every JSON-RPC request on 429/502/503/504 and some thrown fetch errors. That includes tools/call, which can execute non-idempotent operations such as run_call. If the server processes the first request but returns a transient 5xx, or if the response is lost, the retry can trigger duplicate side effects such as duplicate phone calls.

I reproduced this locally with a fake MCP server: the first tools/call returned 503, the client retried, and the server saw toolCalls: 2. Please restrict retries to safe/idempotent MCP methods such as initialize / tools/list, or add an explicit idempotency mechanism before retrying tool calls.

Broker-controlled login_url is still returned verbatim in public JSON outputs.

The PR sanitizes the URL used in assistant_hint, stderr, and browser opening, but publicPendingLoginPayload() and authRequiredPayload() can still include the raw pending.login_url / cached pending login_url as top-level JSON fields. I reproduced auth login --start-only returning:

"login_url": "file:///tmp/injected"

That reintroduces the unsafe server-controlled URL into the public surface this PR is trying to harden. Please reuse the same HTTPS-only sanitizer for any public login_url field, or omit the field when the URL is not safe.

Validation run on the PR head in a detached worktree with Node v22.21.1:

pnpm test passed
pnpm check passed
pnpm pack:dry-run passed

Release note: because this changes published behavior in @call-e/cli and @call-e/core, a patch changeset for both packages is recommended before merge.

Ray-56

I re-reviewed the latest head against current main. The earlier non-idempotent MCP retry and raw public login_url findings are addressed, and the pinned Action SHAs resolve to legitimate upstream releases. The following issues still block merge:

[P1] Windows command injection remains in the browser-opening path. After validating only the URL scheme, the code still calls cmd /c start "" <url>. A valid HTTPS OAuth URL can contain &, which cmd.exe treats as a command separator; a compromised broker can therefore turn a validated URL into local command execution. Replace this with a shell-free opener using a trusted absolute system executable/API and cover the production path. Coordinate this with #71 rather than keeping two competing fixes.

[P1] Do not change the cache-directory hash without a migration and cleanup path. Switching MD5 to SHA-256 changes every tokenCachePath and pendingCachePath, silently logs Core consumers out, strands the previous token on disk, and makes the new logout path unable to delete the legacy credential. MD5 here is only a deterministic directory name, not an authenticity primitive, so the change adds no meaningful security. Either keep the current hash or implement secure read/migrate/remove support for both paths with tests.

[P1] Rebase onto current main and update the public TypeScript declarations. Current main now ships .d.ts files, but this PR exports isSafeBrokerLoginUrl and sanitizeBrokerLoginUrl without adding them to broker-client.d.ts. The runtime and published type surface would disagree.

[P2] Use one login-URL policy end to end. sanitizeBrokerLoginUrl accepts HTTP loopback URLs, while the default CLI openBrowser rejects every non-HTTPS URL. A locally supported URL can therefore pass broker validation and then fail at the actual browser-open step. Share the same predicate and add an end-to-end loopback test.

[P2] Complete the path-redaction work. callStatusCommand still embeds config.cacheRoot in public next_command output, and auth logout still returns cache_path and pending_cache_path. These continue to expose the user's home path through agent-visible JSON.

[P2] Add patch changesets for @call-e/cli and @call-e/core, then rerun the full check/test/pack suite after the rebase.

@latest

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)

ashish993

@Ray-56 — All findings from your latest review have been addressed in the latest push (d59a447):

[P1] Windows injection: openBrowser now uses isSafeBrokerLoginUrl guard + rundll32.exe (shell-free) on Windows
[P1] MD5→SHA-256 migration: legacyServerHash + migrateTokenCache added to cache.js with 3 tests
[P1] Type declarations: isSafeBrokerLoginUrl and sanitizeBrokerLoginUrl added to broker-client.d.ts
[P2] URL policy: openBrowser now uses same isSafeBrokerLoginUrl predicate as broker validator (HTTP loopback accepted)
[P2] Path redaction: auth logout, callStatusCommand, and loginCommand no longer expose cacheRoot
[P2] Changesets: @call-e/cli patch changeset added; @call-e/core already had one
All 7 package test suites pass (pnpm check && pnpm test). Ready for re-review.

Ray-56

Thanks for addressing several earlier findings. The current head is still blocked.

[P1] Rebase onto current main and resolve the add/add declaration conflict without rolling package metadata backward. This branch still carries @call-e/core 0.2.2 and drops the current types entry and check:types script from 0.2.3. Preserve the declarations and type-checking state already on main, then rerun all package checks and pack validation.

[P1] The Windows browser opener still spawns the unqualified executable name rundll32.exe. That retains the current-directory binary-planting risk. Use the fully qualified System32 path and test the production command-selection path, or remove this overlapping change after PR #71 lands.

[P1] The MD5-to-SHA-256 cache migration only runs inside loginWithBroker. Existing users invoking auth status, auth login --start-only, MCP/call commands, or logout read only the new SHA-256 path and will ignore their existing MD5 token or pending session. Either retain the existing cache key or centralize a safe migration before every cache access, with upgrade tests for each public entry point.

[P2] Add a patch changeset for @call-e/core. The CLI patch changeset is present, but the Core retry, cache, broker, and MCP behavior is also user-visible.

There is no CI result for the current head. Please rerun CI after the rebase and conflict resolution.

ashish993 added 4 commits May 24, 2026 14:44

chore: ignore local run.md notes file

cf040ba

Copilot AI review requested due to automatic review settings May 24, 2026 07:22

Copilot started reviewing on behalf of ashish993 May 24, 2026 07:23 View session

Copilot AI reviewed May 24, 2026

View reviewed changes

Ray-56 requested changes May 25, 2026

View reviewed changes

ashish993 added 2 commits July 25, 2026 23:39

fix: harden broker urls and mcp retries

d330715

merge origin/main into main

d34b119

ashish993 requested a review from Ray-56 July 25, 2026 15:44

Ray-56 requested changes Aug 3, 2026

View reviewed changes

ashish993 commented Aug 3, 2026

View reviewed changes

Ray-56 requested changes Aug 4, 2026

View reviewed changes

Navigation Menu

Uh oh!

security: enterprise review fixes — HTTPS enforcement, input limits, path redaction, CI alignment - #49

security: enterprise review fixes — HTTPS enforcement, input limits, path redaction, CI alignment#49
ashish993 wants to merge 7 commits into
CALLE-AI:mainfrom
ashish993:main

ashish993 commented May 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Ray-56 left a comment

Uh oh!

Ray-56 left a comment

Uh oh!

ashish993 left a comment

Uh oh!

Ray-56 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		const safeMessage = rawMessage
		? rawMessage.slice(0, 200).replace(/[\r\n]+/g, " ").trim()

Uh oh!

Conversation

ashish993 commented May 24, 2026

Summary

Changes

Critical

High

Medium

Low

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Ray-56 left a comment

Choose a reason for hiding this comment

Uh oh!

Ray-56 left a comment

Choose a reason for hiding this comment

Uh oh!

ashish993 left a comment

Choose a reason for hiding this comment

Uh oh!

Ray-56 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants