feat(a2a): implement robust IP validation, secure acknowledgement, and URL splitting fix

package-lock.json

@@ -17523,6 +17523,7 @@
         "html-to-text": "^9.0.5",
         "https-proxy-agent": "^7.0.6",
         "ignore": "^7.0.0",
+        "ipaddr.js": "^1.9.1",
         "js-yaml": "^4.1.1",
         "marked": "^15.0.12",
         "mime": "4.0.7",

packages/core/package.json

@@ -65,6 +65,7 @@
     "html-to-text": "^9.0.5",
     "https-proxy-agent": "^7.0.6",
     "ignore": "^7.0.0",
+    "ipaddr.js": "^1.9.1",
     "js-yaml": "^4.1.1",
     "marked": "^15.0.12",
     "mime": "4.0.7",

packages/core/src/agents/a2a-client-manager.test.ts

@@ -419,7 +419,7 @@ describe('A2AClientManager', () => {
 
       expect(resolverInstance.resolve).toHaveBeenCalledWith(
         'http://localhost:9001/',
-        '.well-known/agent-card.json',
+        undefined,
       );
     });
 
@@ -481,43 +481,5 @@ describe('A2AClientManager', () => {
         /contains transport URL pointing to private IP range: http:\/\/192.168.1.1\/api/,
       );
     });
-
-    it('should handle URLs where .well-known appears in the domain/subdomain', async () => {
-      const trickyUrl =
-        'http://.well-known/agent-card.json.attacker.com/my-agent';
-      const resolverInstance = {
-        resolve: vi.fn().mockResolvedValue({ name: 'test' } as AgentCard),
-      };
-      vi.mocked(sdkClient.DefaultAgentCardResolver).mockReturnValue(
-        resolverInstance as unknown as sdkClient.DefaultAgentCardResolver,
-      );
-
-      await manager.loadAgent('tricky-agent', trickyUrl);
-
-      // Should treat the whole thing as baseUrl since it doesn't end with standardPath
-      expect(resolverInstance.resolve).toHaveBeenCalledWith(
-        trickyUrl,
-        undefined,
-      );
-    });
-
-    it('should correctly handle URLs with standardPath in the hash fragment', async () => {
-      const fragmentUrl =
-        'http://localhost:9001/.well-known/agent-card.json#.well-known/agent-card.json';
-      const resolverInstance = {
-        resolve: vi.fn().mockResolvedValue({ name: 'test' } as AgentCard),
-      };
-      vi.mocked(sdkClient.DefaultAgentCardResolver).mockReturnValue(
-        resolverInstance as unknown as sdkClient.DefaultAgentCardResolver,
-      );
-
-      await manager.loadAgent('fragment-agent', fragmentUrl);
-
-      // Should correctly ignore the hash fragment and use the path from the URL object
-      expect(resolverInstance.resolve).toHaveBeenCalledWith(
-        'http://localhost:9001/',
-        '.well-known/agent-card.json',
-      );
-    });
   });
 });

packages/core/src/agents/a2a-client-manager.ts

@@ -29,6 +29,7 @@ import {
   getGrpcCredentials,
   normalizeAgentCard,
   pinUrlToIp,
+  splitAgentCardUrl,
 } from './a2aUtils.js';
 import {
   isPrivateIpAsync,
@@ -291,10 +292,6 @@ export class A2AClientManager {
     url: string,
     resolver: DefaultAgentCardResolver,
   ): Promise<AgentCard> {
-    const standardPath = '.well-known/agent-card.json';
-    let baseUrl = url;
-    let path: string | undefined;
-
     // Validate URL to prevent SSRF (with DNS resolution)
     if (await isPrivateIpAsync(url)) {
       // Local/private IPs are allowed ONLY for localhost for testing.
@@ -306,24 +303,7 @@ export class A2AClientManager {
       }
     }
 
-    try {
-      const parsedUrl = new URL(url);
-      if (parsedUrl.pathname.endsWith(standardPath)) {
-        // Correctly split the URL into baseUrl and standard path
-        path = standardPath;
-        // Reconstruct baseUrl from parsed components to avoid issues with hashes or query params.
-        parsedUrl.pathname = parsedUrl.pathname.substring(
-          0,
-          parsedUrl.pathname.lastIndexOf(standardPath),
-        );
-        parsedUrl.search = '';
-        parsedUrl.hash = '';
-        baseUrl = parsedUrl.toString();
-      }
-    } catch (e) {
-      throw new Error(`Invalid agent card URL: ${url}`, { cause: e });
-    }
-
+    const { baseUrl, path } = splitAgentCardUrl(url);
     const rawCard = await resolver.resolve(baseUrl, path);
     const agentCard = normalizeAgentCard(rawCard);
 

packages/core/src/agents/a2aUtils.test.ts

@@ -14,6 +14,7 @@ import {
   normalizeAgentCard,
   getGrpcCredentials,
   pinUrlToIp,
+  splitAgentCardUrl,
 } from './a2aUtils.js';
 import type { SendMessageResult } from './a2a-client-manager.js';
 import type {
@@ -423,6 +424,60 @@ describe('a2aUtils', () => {
         'http://127.0.0.1:8080',
       );
     });
+
+    it('should NOT override existing transport if protocolBinding is also present', () => {
+      const raw = {
+        name: 'priority-test',
+        supportedInterfaces: [
+          { url: 'foo', transport: 'GRPC', protocolBinding: 'REST' },
+        ],
+      };
+      const normalized = normalizeAgentCard(raw);
+      expect(normalized.additionalInterfaces?.[0].transport).toBe('GRPC');
+    });
+  });
+
+  describe('splitAgentCardUrl', () => {
+    const standard = '.well-known/agent-card.json';
+
+    it('should return baseUrl as-is if it does not end with standard path', () => {
+      const url = 'http://localhost:9001/custom/path';
+      expect(splitAgentCardUrl(url)).toEqual({ baseUrl: url });
+    });
+
+    it('should split correctly if URL ends with standard path', () => {
+      const url = `http://localhost:9001/${standard}`;
+      expect(splitAgentCardUrl(url)).toEqual({
+        baseUrl: 'http://localhost:9001/',
+        path: undefined,
+      });
+    });
+
+    it('should handle trailing slash in baseUrl when splitting', () => {
+      const url = `http://example.com/api/${standard}`;
+      expect(splitAgentCardUrl(url)).toEqual({
+        baseUrl: 'http://example.com/api/',
+        path: undefined,
+      });
+    });
+
+    it('should ignore hashes and query params when splitting', () => {
+      const url = `http://localhost:9001/${standard}?foo=bar#baz`;
+      expect(splitAgentCardUrl(url)).toEqual({
+        baseUrl: 'http://localhost:9001/',
+        path: undefined,
+      });
+    });
+
+    it('should return original URL if parsing fails', () => {
+      const url = 'not-a-url';
+      expect(splitAgentCardUrl(url)).toEqual({ baseUrl: url });
+    });
+
+    it('should handle standard path appearing earlier in the path', () => {
+      const url = `http://localhost:9001/${standard}/something-else`;
+      expect(splitAgentCardUrl(url)).toEqual({ baseUrl: url });
+    });
   });
 
   describe('A2AResultReassembler', () => {

packages/core/src/agents/a2aUtils.ts

@@ -340,6 +340,35 @@ export async function pinUrlToIp(
   }
 }
 
+/**
+ * Splts an agent card URL into a baseUrl and a standard path if it already
+ * contains '.well-known/agent-card.json'.
+ */
+export function splitAgentCardUrl(url: string): {
+  baseUrl: string;
+  path?: string;
+} {
+  const standardPath = '.well-known/agent-card.json';
+  try {
+    const parsedUrl = new URL(url);
+    if (parsedUrl.pathname.endsWith(standardPath)) {
+      // Reconstruct baseUrl from parsed components to avoid issues with hashes or query params.
+      parsedUrl.pathname = parsedUrl.pathname.substring(
+        0,
+        parsedUrl.pathname.lastIndexOf(standardPath),
+      );
+      parsedUrl.search = '';
+      parsedUrl.hash = '';
+      // We return undefined for path if it's the standard one,
+      // because the SDK's DefaultAgentCardResolver appends it automatically.
+      return { baseUrl: parsedUrl.toString(), path: undefined };
+    }
+  } catch (_e) {
+    // Ignore URL parsing errors here, let the resolver handle them.
+  }
+  return { baseUrl: url };
+}
+
 /**
  * Extracts contextId and taskId from a Message, Task, or Update response.
  * Follows the pattern from the A2A CLI sample to maintain conversational continuity.

packages/core/src/agents/registry.test.ts

@@ -5,6 +5,7 @@
  */
 
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import * as crypto from 'node:crypto';
 import { AgentRegistry, getModelConfigAlias } from './registry.js';
 import { makeFakeConfig } from '../test-utils/config.js';
 import type { AgentDefinition, LocalAgentDefinition } from './types.js';
@@ -29,10 +30,21 @@ import { SimpleExtensionLoader } from '../utils/extensionLoader.js';
 import type { ToolRegistry } from '../tools/tool-registry.js';
 import { ThinkingLevel } from '@google/genai';
 import type { AcknowledgedAgentsService } from './acknowledgedAgents.js';
+import * as sdkClient from '@a2a-js/sdk/client';
 import { PolicyDecision } from '../policy/types.js';
 import { A2AAuthProviderFactory } from './auth-provider/factory.js';
 import type { A2AAuthProvider } from './auth-provider/types.js';
 
+vi.mock('@a2a-js/sdk/client', async (importOriginal) => {
+  const actual = await importOriginal();
+  return {
+    ...(actual as Record<string, unknown>),
+    DefaultAgentCardResolver: vi.fn().mockImplementation(() => ({
+      resolve: vi.fn().mockResolvedValue({ name: 'RemoteAgent' }),
+    })),
+  };
+});
+
 vi.mock('./agentLoader.js', () => ({
   loadAgentsFromDirectory: vi
     .fn()
@@ -417,7 +429,7 @@ describe('AgentRegistry', () => {
       expect(registry.getDefinition('extension-agent')).toBeUndefined();
     });
 
-    it('should use agentCardUrl as hash for acknowledgement of remote agents', async () => {
+    it('should use agentCardUrl and content-based hash for acknowledgement of remote agents', async () => {
       mockConfig = makeMockedConfig({ enableAgents: true });
       // Trust the folder so it attempts to load project agents
       vi.spyOn(mockConfig, 'isTrustedFolder').mockReturnValue(true);
@@ -453,19 +465,33 @@ describe('AgentRegistry', () => {
         clearCache: vi.fn(),
       } as unknown as A2AClientManager);
 
+      // Mock the resolver to return a consistent card content for hashing
+      const mockCardContent = { name: 'RemoteAgent' };
+      const expectedContentHash = crypto
+        .createHash('sha256')
+        .update(JSON.stringify(mockCardContent))
+        .digest('hex');
+      const expectedHash = `https://example.com/card#${expectedContentHash}`;
+
+      vi.mocked(sdkClient.DefaultAgentCardResolver).mockImplementation(
+        () =>
+          ({
+            resolve: vi.fn().mockResolvedValue(mockCardContent),
+          }) as unknown as sdkClient.DefaultAgentCardResolver,
+      );
+
       await registry.initialize();
 
-      // Verify ackService was called with the URL, not the file hash
+      // Verify ackService was called with the content-based hash
       expect(ackService.isAcknowledged).toHaveBeenCalledWith(
         expect.anything(),
         'RemoteAgent',
-        'https://example.com/card',
+        expectedHash,
       );
 
-      // Also verify that the agent's metadata was updated to use the URL as hash
-      // Use getDefinition because registerAgent might have been called
+      // Also verify that the agent's metadata was updated to use the content-based hash
       expect(registry.getDefinition('RemoteAgent')?.metadata?.hash).toBe(
-        'https://example.com/card',
+        expectedHash,
       );
     });
   });

packages/core/src/agents/registry.ts

@@ -9,14 +9,17 @@ import { CoreEvent, coreEvents } from '../utils/events.js';
 import type { AgentOverride, Config } from '../config/config.js';
 import type { AgentDefinition, LocalAgentDefinition } from './types.js';
 import { loadAgentsFromDirectory } from './agentLoader.js';
+import { splitAgentCardUrl } from './a2aUtils.js';
 import { CodebaseInvestigatorAgent } from './codebase-investigator.js';
 import { CliHelpAgent } from './cli-help-agent.js';
 import { GeneralistAgent } from './generalist-agent.js';
 import { BrowserAgentDefinition } from './browser/browserAgentDefinition.js';
 import { A2AClientManager } from './a2a-client-manager.js';
 import { A2AAuthProviderFactory } from './auth-provider/factory.js';
+import { DefaultAgentCardResolver } from '@a2a-js/sdk/client';
 import type { AuthenticationHandler } from '@a2a-js/sdk/client';
 import { type z } from 'zod';
+import * as crypto from 'node:crypto';
 import { debugLogger } from '../utils/debugLogger.js';
 import { isAutoModel } from '../config/models.js';
 import {
@@ -155,13 +158,35 @@ export class AgentRegistry {
       const agentsToRegister: AgentDefinition[] = [];
 
       for (const agent of projectAgents.agents) {
-        // If it's a remote agent, use the agentCardUrl as the hash.
-        // This allows multiple remote agents in a single file to be tracked independently.
+        // For remote agents, we must use a content-based hash of the AgentCard
+        // to prevent Indirect Prompt Injection if the remote card is modified.
         if (agent.kind === 'remote') {
-          if (!agent.metadata) {
-            agent.metadata = {};
+          try {
+            // We use a dedicated resolver here to fetch the card for hashing.
+            // This is separate from loadAgent to keep hashing logic isolated.
+            const resolver = new DefaultAgentCardResolver();
+            const { baseUrl, path } = splitAgentCardUrl(agent.agentCardUrl);
+            const rawCard = await resolver.resolve(baseUrl, path);
+            const cardContent = JSON.stringify(rawCard);
+            const contentHash = crypto
+              .createHash('sha256')
+              .update(cardContent)
+              .digest('hex');
+
+            if (!agent.metadata) {
+              agent.metadata = {};
+            }
+            // Combining URL and content hash ensures we track specific card versions at specific locations.
+            agent.metadata.hash = `${agent.agentCardUrl}#${contentHash}`;
+          } catch (e) {
+            debugLogger.warn(
+              `[AgentRegistry] Could not fetch remote card for hashing "${agent.name}":`,
+              e,
+            );
+            // If we can't fetch the card, we can't verify its acknowledgement securely.
+            unacknowledgedAgents.push(agent);
+            continue;
           }
-          agent.metadata.hash = agent.agentCardUrl;
         }
 
         if (!agent.metadata?.hash) {
@@ -178,6 +203,10 @@ export class AgentRegistry {
         if (isAcknowledged) {
           agentsToRegister.push(agent);
         } else {
+          // Register unacknowledged agents so they are visible to the LLM.
+          // They will be registered with ASK_USER policy, triggering the
+          // acknowledgement flow when the LLM tries to call them.
+          agentsToRegister.push(agent);
           unacknowledgedAgents.push(agent);
         }
       }

packages/core/src/utils/fetch.test.ts

@@ -51,7 +51,10 @@ describe('fetch utils', () => {
       expect(isAddressPrivate('192.0.0.1')).toBe(true);
       expect(isAddressPrivate('192.0.2.1')).toBe(true);
       expect(isAddressPrivate('192.88.99.1')).toBe(true);
+      // Benchmark range (198.18.0.0/15)
+      expect(isAddressPrivate('198.18.0.0')).toBe(true);
       expect(isAddressPrivate('198.18.0.1')).toBe(true);
+      expect(isAddressPrivate('198.19.255.255')).toBe(true);
       expect(isAddressPrivate('198.51.100.1')).toBe(true);
       expect(isAddressPrivate('203.0.113.1')).toBe(true);
       expect(isAddressPrivate('224.0.0.1')).toBe(true);

packages/core/src/utils/fetch.ts

@@ -9,6 +9,7 @@ import { URL } from 'node:url';
 import * as dns from 'node:dns';
 import { lookup } from 'node:dns/promises';
 import { Agent, ProxyAgent, setGlobalDispatcher } from 'undici';
+import ipaddr from 'ipaddr.js';
 
 const DEFAULT_HEADERS_TIMEOUT = 300000; // 5 minutes
 const DEFAULT_BODY_TIMEOUT = 300000; // 5 minutes
@@ -21,26 +22,6 @@ setGlobalDispatcher(
   }),
 );
 
-const PRIVATE_IP_RANGES = [
-  /^10\./,
-  /^127\./,
-  /^0\./,
-  /^100\.(6[4-9]|[7-9][0-9]|1[0-1][0-9]|12[0-7])\./,
-  /^169\.254\./,
-  /^172\.(1[6-9]|2[0-9]|3[0-1])\./,
-  /^192\.0\.(0|2)\./,
-  /^192\.88\.99\./,
-  /^192\.168\./,
-  /^198\.(1[8-9]|51\.100)\./,
-  /^203\.0\.113\./,
-  /^2(2[4-9]|3[0-9]|[4-5][0-9])\./,
-  /^::1$/,
-  /^::$/,
-  /^f[cd]00:/i, // fc00::/7 (ULA)
-  /^fe[89ab][0-9a-f]:/i, // fe80::/10 (Link-local)
-  /^::ffff:(10\.|127\.|0\.|100\.(6[4-9]|[7-9][0-9]|1[0-1][0-9]|12[0-7])\.|169\.254\.|172\.(1[6-9]|2[0-9]|3[0-1])\.|192\.0\.(0|2)\.|192\.88\.99\.|192\.168\.|198\.(1[8-9]|51\.100)\.|203\.0\.113\.|2(2[4-9]|3[0-9]|[4-5][0-9])\.|a9fe:|ac1[0-9a-f]:|c0a8:|0+:)/i, // IPv4-mapped IPv6
-];
-
 // Local extension of RequestInit to support Node.js/undici dispatcher
 interface NodeFetchInit extends RequestInit {
   dispatcher?: Agent | ProxyAgent;
@@ -166,15 +147,43 @@ export async function isPrivateIpAsync(url: string): Promise<boolean> {
 }
 
 /**
- * Internal helper to check if an IP address string is in a private range.
+ * Internal helper to check if an IP address string is in a private or reserved range.
  */
 export function isAddressPrivate(address: string): boolean {
   const sanitized = sanitizeHostname(address);
 
-  return (
-    sanitized === 'localhost' ||
-    PRIVATE_IP_RANGES.some((range) => range.test(sanitized))
-  );
+  if (sanitized === 'localhost') {
+    return true;
+  }
+
+  try {
+    if (!ipaddr.isValid(sanitized)) {
+      return false;
+    }
+
+    const addr = ipaddr.parse(sanitized);
+    const range = addr.range();
+
+    // Special handling for IPv4-mapped IPv6 (::ffff:x.x.x.x)
+    // We unmap it and check the underlying IPv4 address.
+    if (addr instanceof ipaddr.IPv6 && addr.isIPv4MappedAddress()) {
+      return isAddressPrivate(addr.toIPv4Address().toString());
+    }
+
+    // Explicitly block 198.18.0.0/15 (Benchmark testing) which ipaddr.js
+    // classifies as unicast, but is not public internet.
+    if (addr instanceof ipaddr.IPv4) {
+      const [r, bits] = ipaddr.parseCIDR('198.18.0.0/15');
+      if (r instanceof ipaddr.IPv4 && addr.match(r, bits)) {
+        return true;
+      }
+    }
+
+    return range !== 'unicast';
+  } catch (_e) {
+    // If parsing fails despite isValid(), we treat it as potentially unsafe.
+    return true;
+  }
 }
 
 /**