diff --git a/src/__tests__/security-hardening.test.ts b/src/__tests__/security-hardening.test.ts index 45c6e5d9..c32cb62a 100644 --- a/src/__tests__/security-hardening.test.ts +++ b/src/__tests__/security-hardening.test.ts @@ -124,3 +124,68 @@ describe('WebFetch SSRF guard', () => { expect(fnSection).toContain('lookup: ssrfGuardedLookup') }) }) + +// --------------------------------------------------------------------------- +// Fix 6: Swarm permission file polling removed (security hardening) +// --------------------------------------------------------------------------- +describe('Swarm permission file polling removed', () => { + test('useSwarmPermissionPoller hook no longer exists', async () => { + const content = await file( + 'hooks/useSwarmPermissionPoller.ts', + ).text() + // The file-based polling hook must not exist — it read from an + // unauthenticated resolved/ directory where any local process could + // forge approval files. + expect(content).not.toContain('function useSwarmPermissionPoller(') + // The file-based processResponse must not exist + expect(content).not.toContain('function processResponse(') + }) + + test('poller does not import from permissionSync', async () => { + const content = await file( + 'hooks/useSwarmPermissionPoller.ts', + ).text() + // Must not import anything from permissionSync — all file-based + // functions have been removed from this module's dependencies + expect(content).not.toContain('permissionSync') + }) + + test('file-based permission functions are marked deprecated', async () => { + const content = await file( + 'utils/swarm/permissionSync.ts', + ).text() + // All file-based functions must have @deprecated JSDoc + const deprecatedFns = [ + 'writePermissionRequest', + 'readPendingPermissions', + 'readResolvedPermission', + 'resolvePermission', + 'pollForResponse', + 'removeWorkerResponse', + ] + for (const fn of deprecatedFns) { + // Find the function and check that @deprecated appears before it + const fnIndex = content.indexOf(`export async function ${fn}(`) + if (fnIndex === -1) continue // submitPermissionRequest is a const, not async function + const preceding = content.slice(Math.max(0, fnIndex - 500), fnIndex) + expect(preceding).toContain('@deprecated') + } + }) + + test('mailbox-based functions are NOT deprecated', async () => { + const content = await file( + 'utils/swarm/permissionSync.ts', + ).text() + // These are the active path — must not be deprecated + const activeFns = [ + 'sendPermissionRequestViaMailbox', + 'sendPermissionResponseViaMailbox', + ] + for (const fn of activeFns) { + const fnIndex = content.indexOf(`export async function ${fn}(`) + expect(fnIndex).not.toBe(-1) + const preceding = content.slice(Math.max(0, fnIndex - 300), fnIndex) + expect(preceding).not.toContain('@deprecated') + } + }) +}) \ No newline at end of file diff --git a/src/hooks/useSwarmPermissionPoller.ts b/src/hooks/useSwarmPermissionPoller.ts index 0223cef1..710aa141 100644 --- a/src/hooks/useSwarmPermissionPoller.ts +++ b/src/hooks/useSwarmPermissionPoller.ts @@ -1,34 +1,23 @@ /** - * Swarm Permission Poller Hook + * Swarm Permission Callback Registry * - * This hook polls for permission responses from the team leader when running - * as a worker agent in a swarm. When a response is received, it calls the - * appropriate callback (onAllow/onReject) to continue execution. + * Manages callback registrations for permission requests and responses + * in agent swarms. Responses are delivered exclusively via the mailbox + * system (useInboxPoller → processMailboxPermissionResponse). * - * This hook should be used in conjunction with the worker-side integration - * in useCanUseTool.ts, which creates pending requests that this hook monitors. + * The legacy file-based polling (resolved/ directory) has been removed + * because it created an unauthenticated attack surface — any local process + * could forge approval files. The mailbox path is the sole active channel. */ -import { useCallback, useEffect, useRef } from 'react' -import { useInterval } from 'usehooks-ts' import { logForDebugging } from '../utils/debug.js' -import { errorMessage } from '../utils/errors.js' import { type PermissionUpdate, permissionUpdateSchema, } from '../utils/permissions/PermissionUpdateSchema.js' -import { - isSwarmWorker, - type PermissionResponse, - pollForResponse, - removeWorkerResponse, -} from '../utils/swarm/permissionSync.js' -import { getAgentName, getTeamName } from '../utils/teammate.js' - -const POLL_INTERVAL_MS = 500 /** - * Validate permissionUpdates from external sources (mailbox IPC, disk polling). + * Validate permissionUpdates from external sources (mailbox IPC). * Malformed entries from buggy/old teammate processes are filtered out rather * than propagated unchecked into callback.onAllow(). */ @@ -225,106 +214,9 @@ export function processSandboxPermissionResponse(params: { return true } -/** - * Process a permission response by invoking the registered callback - */ -function processResponse(response: PermissionResponse): boolean { - const callback = pendingCallbacks.get(response.requestId) - - if (!callback) { - logForDebugging( - `[SwarmPermissionPoller] No callback registered for request ${response.requestId}`, - ) - return false - } - - logForDebugging( - `[SwarmPermissionPoller] Processing response for request ${response.requestId}: ${response.decision}`, - ) - - // Remove from registry before invoking callback - pendingCallbacks.delete(response.requestId) - - if (response.decision === 'approved') { - const permissionUpdates = parsePermissionUpdates(response.permissionUpdates) - const updatedInput = response.updatedInput - callback.onAllow(updatedInput, permissionUpdates) - } else { - callback.onReject(response.feedback) - } - - return true -} - -/** - * Hook that polls for permission responses when running as a swarm worker. - * - * This hook: - * 1. Only activates when isSwarmWorker() returns true - * 2. Polls every 500ms for responses - * 3. When a response is found, invokes the registered callback - * 4. Cleans up the response file after processing - */ -export function useSwarmPermissionPoller(): void { - const isProcessingRef = useRef(false) - - const poll = useCallback(async () => { - // Don't poll if not a swarm worker - if (!isSwarmWorker()) { - return - } - - // Prevent concurrent polling - if (isProcessingRef.current) { - return - } - - // Don't poll if no callbacks are registered - if (pendingCallbacks.size === 0) { - return - } - - isProcessingRef.current = true - - try { - const agentName = getAgentName() - const teamName = getTeamName() - - if (!agentName || !teamName) { - return - } - - // Check each pending request for a response - for (const [requestId, _callback] of pendingCallbacks) { - const response = await pollForResponse(requestId, agentName, teamName) - - if (response) { - // Process the response - const processed = processResponse(response) - - if (processed) { - // Clean up the response from the worker's inbox - await removeWorkerResponse(requestId, agentName, teamName) - } - } - } - } catch (error) { - logForDebugging( - `[SwarmPermissionPoller] Error during poll: ${errorMessage(error)}`, - ) - } finally { - isProcessingRef.current = false - } - }, []) - - // Only poll if we're a swarm worker - const shouldPoll = isSwarmWorker() - useInterval(() => void poll(), shouldPoll ? POLL_INTERVAL_MS : null) - - // Initial poll on mount - useEffect(() => { - if (isSwarmWorker()) { - void poll() - } - }, [poll]) -} +// Legacy file-based polling (useSwarmPermissionPoller, processResponse) +// has been removed. Permission responses are now delivered exclusively +// via the mailbox system: +// Leader: sendPermissionResponseViaMailbox() → writeToMailbox() +// Worker: useInboxPoller → processMailboxPermissionResponse() +// See: fix(security) — remove unauthenticated file-based permission channel diff --git a/src/utils/swarm/permissionSync.ts b/src/utils/swarm/permissionSync.ts index 001e7a26..1ebea56a 100644 --- a/src/utils/swarm/permissionSync.ts +++ b/src/utils/swarm/permissionSync.ts @@ -207,6 +207,10 @@ export function createPermissionRequest(params: { } /** + * @deprecated Use sendPermissionRequestViaMailbox() instead. This file-based + * approach writes to an unauthenticated directory where any local process can + * forge requests. Retained for backward compatibility but no longer called. + * * Write a permission request to the pending directory with file locking * Called by worker agents when they need permission approval from the leader * @@ -250,6 +254,10 @@ export async function writePermissionRequest( } /** + * @deprecated No longer called — permission requests are sent via mailbox. + * The pending directory is an unauthenticated channel. Retained for backward + * compatibility. + * * Read all pending permission requests for a team * Called by the team leader to see what requests need attention */ @@ -312,6 +320,11 @@ export async function readPendingPermissions( } /** + * @deprecated No longer called — permission responses are delivered via mailbox + * (processMailboxPermissionResponse). The resolved directory is an unauthenticated + * channel where any local process can forge approvals. Retained for backward + * compatibility. + * * Read a resolved permission request by ID * Called by workers to check if their request has been resolved * @@ -352,6 +365,10 @@ export async function readResolvedPermission( } /** + * @deprecated Use sendPermissionResponseViaMailbox() instead. This file-based + * approach writes to an unauthenticated directory where any local process can + * forge approvals. Retained for backward compatibility but no longer called. + * * Resolve a permission request * Called by the team leader (or worker in self-resolution cases) * @@ -536,6 +553,10 @@ export type PermissionResponse = { } /** + * @deprecated Use processMailboxPermissionResponse() via useInboxPoller instead. + * File-based polling reads from an unauthenticated directory where any local + * process can forge approval files. Retained for backward compatibility. + * * Poll for a permission response (worker-side convenience function) * Converts the resolved request into a simpler response format * @@ -564,6 +585,9 @@ export async function pollForResponse( } /** + * @deprecated File-based response cleanup is no longer needed — responses are + * delivered via mailbox. Retained for backward compatibility. + * * Remove a worker's response after processing * This is an alias for deleteResolvedPermission for backward compatibility */ @@ -601,6 +625,9 @@ export function isSwarmWorker(): boolean { } /** + * @deprecated File-based resolved permissions are no longer written. Responses + * are delivered via mailbox. Retained for backward compatibility. + * * Delete a resolved permission file * Called after a worker has processed the resolution */ @@ -635,8 +662,8 @@ export async function deleteResolvedPermission( } /** - * Submit a permission request (alias for writePermissionRequest) - * Provided for backward compatibility with worker integration code + * @deprecated Alias for writePermissionRequest, which is itself deprecated. + * Use sendPermissionRequestViaMailbox() instead. */ export const submitPermissionRequest = writePermissionRequest