diff --git a/ts/packages/agentRpc/src/client.ts b/ts/packages/agentRpc/src/client.ts index 83ab24fa7a..1e42e0791c 100644 --- a/ts/packages/agentRpc/src/client.ts +++ b/ts/packages/agentRpc/src/client.ts @@ -257,6 +257,7 @@ export async function createAgentRpcClient( actionContextId: actionContextMap.getId(actionContext), activityContext: actionContext.activityContext, isFromReasoningLoop: actionContext.isFromReasoningLoop, + workingDirectory: actionContext.workingDirectory, ...getContextParam(actionContext.sessionContext), }); } finally { @@ -265,15 +266,14 @@ export async function createAgentRpcClient( } async function withActionContextAsync( actionContext: ActionContext, - fn: (contextParams: { - actionContextId: number; - isFromReasoningLoop: boolean; - }) => Promise, + fn: (contextParams: ActionContextParams) => Promise, ) { try { return await fn({ actionContextId: actionContextMap.getId(actionContext), + activityContext: actionContext.activityContext, isFromReasoningLoop: actionContext.isFromReasoningLoop, + workingDirectory: actionContext.workingDirectory, ...getContextParam(actionContext.sessionContext), }); } finally { diff --git a/ts/packages/agentRpc/src/server.ts b/ts/packages/agentRpc/src/server.ts index 625189c10d..38b7a54c59 100644 --- a/ts/packages/agentRpc/src/server.ts +++ b/ts/packages/agentRpc/src/server.ts @@ -816,6 +816,7 @@ export function createAgentRpcServer( streamingContext: undefined, activityContext: param.activityContext, isFromReasoningLoop: param.isFromReasoningLoop ?? false, + workingDirectory: param.workingDirectory, get abortSignal() { return abortController.signal; }, diff --git a/ts/packages/agentRpc/src/types.ts b/ts/packages/agentRpc/src/types.ts index e4632026c6..9136dd7318 100644 --- a/ts/packages/agentRpc/src/types.ts +++ b/ts/packages/agentRpc/src/types.ts @@ -304,6 +304,7 @@ export type ActionContextParams = ContextParams & { actionContextId: number; activityContext: ActivityContext | undefined; isFromReasoningLoop: boolean; + workingDirectory: string | undefined; }; export type OptionsFunctionCallBack = { diff --git a/ts/packages/agentRpc/test/actionContext.spec.ts b/ts/packages/agentRpc/test/actionContext.spec.ts new file mode 100644 index 0000000000..85c3349728 --- /dev/null +++ b/ts/packages/agentRpc/test/actionContext.spec.ts @@ -0,0 +1,84 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { + ActionContext, + AppAgent, + SessionContext, +} from "@typeagent/agent-sdk"; +import { createAgentRpcClient } from "../src/client.js"; +import { + createChannelProviderAdapter, + type ChannelProviderAdapter, +} from "../src/common.js"; +import { createAgentRpcServer } from "../src/server.js"; + +describe("agent action context RPC", () => { + test("propagates workingDirectory to the out-of-process agent", async () => { + let clientProvider: ChannelProviderAdapter; + let serverProvider: ChannelProviderAdapter; + clientProvider = createChannelProviderAdapter( + "test-client", + (message, callback) => { + queueMicrotask(() => serverProvider.notifyMessage(message)); + callback?.(null); + }, + ); + serverProvider = createChannelProviderAdapter( + "test-server", + (message, callback) => { + queueMicrotask(() => clientProvider.notifyMessage(message)); + callback?.(null); + }, + ); + + let receivedWorkingDirectory: string | undefined; + const serverAgent: AppAgent = { + initializeAgentContext: async () => ({}), + executeAction: async (_action, context) => { + receivedWorkingDirectory = context.workingDirectory; + return undefined; + }, + }; + const server = createAgentRpcServer( + "test", + serverAgent, + serverProvider, + ); + const clientAgent = await createAgentRpcClient( + "test", + clientProvider, + server.agentInterface, + ); + + try { + const agentContext = await clientAgent.initializeAgentContext?.(); + const sessionContext = { + agentContext, + sessionContextId: "rpc-working-directory-test", + } as SessionContext; + const actionContext = { + sessionContext, + workingDirectory: "C:\\host-authorized-workspace", + isFromReasoningLoop: false, + } as ActionContext; + + await clientAgent.executeAction?.( + { + schemaName: "test", + actionName: "test", + parameters: {}, + }, + actionContext, + ); + + expect(receivedWorkingDirectory).toBe( + "C:\\host-authorized-workspace", + ); + } finally { + server.closeFn(); + clientProvider.notifyDisconnected(); + serverProvider.notifyDisconnected(); + } + }); +}); diff --git a/ts/packages/agentSdk/src/agentInterface.ts b/ts/packages/agentSdk/src/agentInterface.ts index 568e40ca8e..cbab7a91c1 100644 --- a/ts/packages/agentSdk/src/agentInterface.ts +++ b/ts/packages/agentSdk/src/agentInterface.ts @@ -473,6 +473,9 @@ export interface ActionContext { // to execute immediately or redirect back to the reasoning loop. readonly isFromReasoningLoop: boolean; + // Absolute filesystem root authorized by the host for this action. + readonly workingDirectory?: string | undefined; + // queue up toggle transient agent to be executed at the end of the commands queueToggleTransientAgent( agentName: string, diff --git a/ts/packages/agents/markdown/src/agent/markdownActionHandler.ts b/ts/packages/agents/markdown/src/agent/markdownActionHandler.ts index 6882347c46..863a2412b9 100644 --- a/ts/packages/agents/markdown/src/agent/markdownActionHandler.ts +++ b/ts/packages/agents/markdown/src/agent/markdownActionHandler.ts @@ -11,14 +11,25 @@ import { AppAgentInitSettings, } from "@typeagent/agent-sdk"; import { createActionResult } from "@typeagent/agent-sdk/helpers/action"; -import { MarkdownAction } from "./markdownActionSchema.js"; +import { + CreateDocumentAction, + MarkdownAction, + OpenDocumentAction, +} from "./markdownActionSchema.js"; import { DocumentOperation } from "./markdownOperationSchema.js"; import { createMarkdownAgent } from "./translator.js"; import { ChildProcess, fork } from "child_process"; +import fs from "node:fs"; import { fileURLToPath } from "node:url"; import path from "node:path"; import { UICommandResult } from "./ipcTypes.js"; import registerDebug from "debug"; +import { + normalizeRelativeDocumentPath, + resolveExistingFileWithinRoot, + resolveRealDirectory, + resolveWritableFileWithinRoot, +} from "./pathPolicy.js"; const debug = registerDebug("typeagent:markdown:agent"); @@ -43,8 +54,19 @@ async function executeMarkdownAction( return result; } +type CurrentMarkdownDocument = + | { + source: "session"; + storageKey: string; + } + | { + source: "workspace"; + filePath: string; + workspaceRoot: string; + }; + type MarkdownActionContext = { - currentFileName?: string | undefined; + currentDocument?: CurrentMarkdownDocument | undefined; viewProcess?: ChildProcess | undefined; localHostPort: number; // Handle returned by sessionContext.registerPort for the markdown @@ -285,23 +307,41 @@ async function updateMarkdownContext( // Store agent context for UI command processing setCurrentAgentContext(context.agentContext); - if (!context.agentContext.currentFileName) { - context.agentContext.currentFileName = "live.md"; + if (context.agentContext.currentDocument === undefined) { + context.agentContext.currentDocument = { + source: "session", + storageKey: "live.md", + }; } const storage = context.sessionStorage; - const fileName = context.agentContext.currentFileName; + const currentDocument = context.agentContext.currentDocument; + const storageKey = + currentDocument.source === "session" + ? currentDocument.storageKey + : undefined; - if (!(await storage?.exists(fileName))) { - await storage?.write(fileName, ""); + if (storageKey && !(await storage?.exists(storageKey))) { + await storage?.write(storageKey, ""); } debug( - `Agent context updated for: ${fileName}, port: ${context.agentContext.localHostPort}`, + `Agent context updated for: ${ + currentDocument.source === "session" + ? currentDocument.storageKey + : currentDocument.filePath + }, port: ${context.agentContext.localHostPort}`, ); - if (!context.agentContext.viewProcess) { - const fullPath = await getFullMarkdownFilePath(fileName, storage!); + if ( + !context.agentContext.viewProcess && + currentDocument.source === "session" && + storage + ) { + const fullPath = await getFullMarkdownFilePath( + currentDocument.storageKey, + storage, + ); if (fullPath) { process.env.MARKDOWN_FILE = fullPath; // Fork the express view service in the background instead of @@ -381,16 +421,16 @@ async function handleStreamingMarkdownAction( const agent = await createMarkdownAgent("GPT_4o"); const storage = actionContext.sessionContext.sessionStorage; + const viewProcess = getCurrentDocumentViewProcess( + actionContext.sessionContext.agentContext, + ); // Get current document content - const filePath = `${actionContext.sessionContext.agentContext.currentFileName}`; let markdownContent = ""; - if (actionContext.sessionContext.agentContext.viewProcess) { + if (viewProcess) { try { - markdownContent = await getDocumentContentFromView( - actionContext.sessionContext.agentContext.viewProcess, - ); + markdownContent = await getDocumentContentFromView(viewProcess); debug( `Got content from view process for streaming: ${markdownContent?.length || 0} chars`, ); @@ -399,14 +439,16 @@ async function handleStreamingMarkdownAction( "[STREAMING] Failed to get content from view, falling back to storage:", error, ); - if (await storage?.exists(filePath)) { - markdownContent = (await storage?.read(filePath, "utf8")) || ""; - } + markdownContent = await getCurrentMarkdownContent( + actionContext.sessionContext.agentContext, + storage, + ); } } else { - if (await storage?.exists(filePath)) { - markdownContent = (await storage?.read(filePath, "utf8")) || ""; - } + markdownContent = await getCurrentMarkdownContent( + actionContext.sessionContext.agentContext, + storage, + ); } try { @@ -491,7 +533,9 @@ function sendStreamingChunkToView( chunk: string, actionContext: ActionContext, ): void { - const viewProcess = actionContext.sessionContext.agentContext.viewProcess; + const viewProcess = getCurrentDocumentViewProcess( + actionContext.sessionContext.agentContext, + ); if (viewProcess) { viewProcess.send({ type: "streamingContent", @@ -512,7 +556,9 @@ function sendStreamingCompleteToView( operations: any[], actionContext: ActionContext, ): void { - const viewProcess = actionContext.sessionContext.agentContext.viewProcess; + const viewProcess = getCurrentDocumentViewProcess( + actionContext.sessionContext.agentContext, + ); if (viewProcess) { viewProcess.send({ type: "streamingComplete", @@ -534,12 +580,251 @@ async function getFullMarkdownFilePath(fileName: string, storage: Storage) { return candidates ? candidates[0] : undefined; } +function getCurrentDocumentViewProcess( + agentContext: MarkdownActionContext, +): ChildProcess | undefined { + return agentContext.currentDocument?.source === "session" + ? agentContext.viewProcess + : undefined; +} + +function getDocumentName(rawName: unknown): string { + const relativeCandidate = normalizeRelativeDocumentPath(rawName); + if (relativeCandidate === undefined) { + throw new Error( + `Document name is not a safe relative path: ${JSON.stringify(rawName)}`, + ); + } + return relativeCandidate.toLowerCase().endsWith(".md") + ? relativeCandidate + : `${relativeCandidate}.md`; +} + +async function getCurrentMarkdownContent( + agentContext: MarkdownActionContext, + storage: Storage | undefined, +): Promise { + const currentDocument = agentContext.currentDocument; + if (currentDocument === undefined) { + return ""; + } + if (currentDocument.source === "workspace") { + try { + return await fs.promises.readFile( + currentDocument.filePath, + "utf-8", + ); + } catch (error) { + if ( + error instanceof Error && + "code" in error && + error.code === "ENOENT" + ) { + throw new Error( + `Current Markdown document no longer exists: ${currentDocument.filePath}`, + ); + } + throw error; + } + } + if ( + storage !== undefined && + (await storage.exists(currentDocument.storageKey)) + ) { + return (await storage.read(currentDocument.storageKey, "utf8")) ?? ""; + } + return ""; +} + +async function handleCreateDocument( + action: CreateDocumentAction, + actionContext: ActionContext, +): Promise { + const rawName = action.parameters.name; + const relativeName = getDocumentName(rawName); + + const initialContent = action.parameters.content ?? ""; + const workingDirectory = actionContext.workingDirectory; + const storage = actionContext.sessionContext.sessionStorage; + const agentContext = actionContext.sessionContext.agentContext; + let documentExisted: boolean; + let absoluteFilePath: string | undefined; + + if (workingDirectory === undefined) { + if (storage === undefined) { + throw new Error( + "Markdown document creation requires a working directory or session storage", + ); + } + + documentExisted = await storage.exists(relativeName); + if (!documentExisted) { + await storage.write(relativeName, initialContent); + } else if (initialContent) { + const existingContent = await storage.read(relativeName, "utf8"); + if (existingContent) { + throw new Error( + `Document ${relativeName} already contains content`, + ); + } + await storage.write(relativeName, initialContent); + } + + agentContext.currentDocument = { + source: "session", + storageKey: relativeName, + }; + + if (agentContext.viewProcess) { + const fullPath = await getFullMarkdownFilePath( + relativeName, + storage, + ); + if (fullPath) { + agentContext.viewProcess.send({ + type: "setFile", + filePath: path.basename(fullPath), + folderPath: path.dirname(fullPath), + }); + } + } + } else { + const canonicalRoot = resolveRealDirectory(workingDirectory); + if (canonicalRoot === undefined) { + throw new Error( + `Configured working directory is not a real directory: ${workingDirectory}`, + ); + } + absoluteFilePath = resolveWritableFileWithinRoot( + canonicalRoot, + relativeName, + ); + if (absoluteFilePath === undefined) { + throw new Error( + `Document path is not writable within the working directory: ${JSON.stringify(rawName)}`, + ); + } + + documentExisted = fs.existsSync(absoluteFilePath); + if (!documentExisted) { + fs.writeFileSync(absoluteFilePath, initialContent, { + encoding: "utf-8", + flag: "wx", + }); + } else if (initialContent) { + const existingContent = fs.readFileSync(absoluteFilePath, "utf-8"); + if (existingContent) { + throw new Error( + `Document ${relativeName} already contains content`, + ); + } + fs.writeFileSync(absoluteFilePath, initialContent, "utf-8"); + } + + agentContext.currentDocument = { + source: "workspace", + filePath: absoluteFilePath, + workspaceRoot: canonicalRoot, + }; + } + + const actionLabel = documentExisted ? "opened" : "created"; + const documentLocation = absoluteFilePath ?? relativeName; + const result = createActionResult( + `Document ${actionLabel} at ${documentLocation}`, + ); + result.resultEntity = { + name: relativeName, + type: ["file", "markdown"], + }; + result.activityContext = { + activityName: "editingMarkdown", + description: "Editing a Markdown document", + state: { + fileName: relativeName, + }, + openLocalView: agentContext.currentDocument.source === "session", + }; + return result; +} + +async function handleOpenDocument( + action: OpenDocumentAction, + actionContext: ActionContext, +): Promise { + const relativeName = getDocumentName(action.parameters.name); + const workingDirectory = actionContext.workingDirectory; + const storage = actionContext.sessionContext.sessionStorage; + const agentContext = actionContext.sessionContext.agentContext; + let documentLocation: string; + + if (workingDirectory === undefined) { + if (storage === undefined || !(await storage.exists(relativeName))) { + throw new Error(`Document does not exist: ${relativeName}`); + } + agentContext.currentDocument = { + source: "session", + storageKey: relativeName, + }; + documentLocation = relativeName; + + if (agentContext.viewProcess) { + const fullPath = await getFullMarkdownFilePath( + relativeName, + storage, + ); + if (fullPath) { + agentContext.viewProcess.send({ + type: "setFile", + filePath: path.basename(fullPath), + }); + } + } + } else { + const canonicalRoot = resolveRealDirectory(workingDirectory); + if (canonicalRoot === undefined) { + throw new Error( + `Configured working directory is not a real directory: ${workingDirectory}`, + ); + } + const absoluteFilePath = resolveExistingFileWithinRoot( + canonicalRoot, + relativeName, + ); + if (absoluteFilePath === undefined) { + throw new Error( + `Document does not exist within the working directory: ${relativeName}`, + ); + } + agentContext.currentDocument = { + source: "workspace", + filePath: absoluteFilePath, + workspaceRoot: canonicalRoot, + }; + documentLocation = absoluteFilePath; + } + + const result = createActionResult(`Document opened at ${documentLocation}`); + result.resultEntity = { + name: relativeName, + type: ["file", "markdown"], + }; + result.activityContext = { + activityName: "editingMarkdown", + description: "Editing a Markdown document", + state: { + fileName: relativeName, + }, + openLocalView: agentContext.currentDocument.source === "session", + }; + return result; +} + async function handleMarkdownAction( action: MarkdownAction, actionContext: ActionContext, ) { let result: ActionResult | undefined = undefined; - const agent = await createMarkdownAgent("GPT_4o"); // Accumulates the LLM token usage consumed while handling this action so // it can be reported back to the dispatcher as "Action Tokens". The agent @@ -549,69 +834,37 @@ async function handleMarkdownAction( completion_tokens: 0, total_tokens: 0, }; - agent.tokenUsage = tokenUsage; + const createAgent = async () => { + const agent = await createMarkdownAgent("GPT_4o"); + agent.tokenUsage = tokenUsage; + return agent; + }; const storage = actionContext.sessionContext.sessionStorage; switch (action.actionName) { - case "openDocument": case "createDocument": { - if (!action.parameters.name) { - result = createActionResult( - "Document could not be created: no name was provided", - ); - } else { - result = createActionResult("Opening document ..."); - - let newFileName = action.parameters.name.trim(); - if (!newFileName.endsWith(".md")) { - newFileName += ".md"; - } - - actionContext.sessionContext.agentContext.currentFileName = - newFileName; - - if (!(await storage?.exists(newFileName))) { - await storage?.write(newFileName, ""); - } - - if (actionContext.sessionContext.agentContext.viewProcess) { - const fullPath = await getFullMarkdownFilePath( - newFileName, - storage!, - ); - - actionContext.sessionContext.agentContext.viewProcess.send({ - type: "setFile", - filePath: path.basename(fullPath!), - folderPath: path.dirname(fullPath!), - }); - } - result = createActionResult("Document opened"); - result.activityContext = { - activityName: "editingMarkdown", - description: "Editing a Markdown document", - state: { - fileName: newFileName, - }, - openLocalView: true, - }; - } + result = await handleCreateDocument(action, actionContext); + break; + } + case "openDocument": { + result = await handleOpenDocument(action, actionContext); break; } case "updateDocument": { + const agent = await createAgent(); debug("Starting updateDocument action in agent process"); result = createActionResult("Updating document ..."); - - const filePath = `${actionContext.sessionContext.agentContext.currentFileName}`; + const viewProcess = getCurrentDocumentViewProcess( + actionContext.sessionContext.agentContext, + ); let markdownContent = ""; - if (actionContext.sessionContext.agentContext.viewProcess) { + if (viewProcess) { try { - markdownContent = await getDocumentContentFromView( - actionContext.sessionContext.agentContext.viewProcess, - ); + markdownContent = + await getDocumentContentFromView(viewProcess); debug( `Got content from view process: ${markdownContent?.length || 0} chars`, ); @@ -620,24 +873,24 @@ async function handleMarkdownAction( ); } catch (error) { console.warn( - "Failed to get content from view, using empty content fallback:", + "Failed to get content from view, reading the current document directly:", error, ); - // Use empty content as fallback to allow agent to continue processing - markdownContent = ""; - debug("Using empty content fallback"); - } - } else { - // Fallback if no view process - if (await storage?.exists(filePath)) { - markdownContent = - (await storage?.read(filePath, "utf8")) || ""; - debug( - "No view process, read content from storage:", - markdownContent?.length, - "chars", + markdownContent = await getCurrentMarkdownContent( + actionContext.sessionContext.agentContext, + storage, ); } + } else { + markdownContent = await getCurrentMarkdownContent( + actionContext.sessionContext.agentContext, + storage, + ); + debug( + "No view process, read current document content:", + markdownContent.length, + "chars", + ); } // Handle synchronous requests through the agent @@ -693,14 +946,13 @@ async function handleMarkdownAction( updateResult.operations.length > 0 ) { // Send operations to view process for application - if (actionContext.sessionContext.agentContext.viewProcess) { + if (viewProcess) { debug( "Agent sending operations to view process for Yjs application", ); const success = await sendOperationsToView( - actionContext.sessionContext.agentContext - .viewProcess, + viewProcess, updateResult.operations, ); @@ -740,21 +992,22 @@ async function handleMarkdownAction( break; } case "streamingUpdateDocument": { + const agent = await createAgent(); // Handle streaming AI commands - now unified with regular updateDocument flow debug( "Starting streamingUpdateDocument action - using standard translator flow", ); result = createActionResult("Updating document ..."); - - const filePath = `${actionContext.sessionContext.agentContext.currentFileName}`; + const viewProcess = getCurrentDocumentViewProcess( + actionContext.sessionContext.agentContext, + ); let markdownContent = ""; - if (actionContext.sessionContext.agentContext.viewProcess) { + if (viewProcess) { try { - markdownContent = await getDocumentContentFromView( - actionContext.sessionContext.agentContext.viewProcess, - ); + markdownContent = + await getDocumentContentFromView(viewProcess); debug( `Got content from view process: ${markdownContent?.length || 0} chars`, ); @@ -763,24 +1016,24 @@ async function handleMarkdownAction( ); } catch (error) { console.warn( - "Failed to get content from view, using empty content fallback:", + "Failed to get content from view, reading the current document directly:", error, ); - // Use empty content as fallback to allow agent to continue processing - markdownContent = ""; - debug("Using empty content fallback"); - } - } else { - // Fallback if no view process - if (await storage?.exists(filePath)) { - markdownContent = - (await storage?.read(filePath, "utf8")) || ""; - debug( - "No view process, read content from storage:", - markdownContent?.length, - "chars", + markdownContent = await getCurrentMarkdownContent( + actionContext.sessionContext.agentContext, + storage, ); } + } else { + markdownContent = await getCurrentMarkdownContent( + actionContext.sessionContext.agentContext, + storage, + ); + debug( + "No view process, read current document content:", + markdownContent.length, + "chars", + ); } // Handle streaming requests through the standard agent (same as updateDocument) @@ -798,14 +1051,13 @@ async function handleMarkdownAction( updateResult.operations.length > 0 ) { // Send operations to view process for application - if (actionContext.sessionContext.agentContext.viewProcess) { + if (viewProcess) { debug( "Agent sending operations to view process for Yjs application", ); const success = await sendOperationsToView( - actionContext.sessionContext.agentContext - .viewProcess, + viewProcess, updateResult.operations, ); diff --git a/ts/packages/agents/markdown/src/agent/markdownActionSchema.ts b/ts/packages/agents/markdown/src/agent/markdownActionSchema.ts index b36433f5bc..a6d56f1dc7 100644 --- a/ts/packages/agents/markdown/src/agent/markdownActionSchema.ts +++ b/ts/packages/agents/markdown/src/agent/markdownActionSchema.ts @@ -13,6 +13,8 @@ export type CreateDocumentAction = { parameters: { // the name to use for the document name: string; + // markdown content to write into the new document + content?: string; }; }; diff --git a/ts/packages/agents/markdown/src/agent/pathPolicy.ts b/ts/packages/agents/markdown/src/agent/pathPolicy.ts new file mode 100644 index 0000000000..c93deaf3e6 --- /dev/null +++ b/ts/packages/agents/markdown/src/agent/pathPolicy.ts @@ -0,0 +1,183 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import fs from "node:fs"; +import path from "node:path"; + +type RootPaths = { + resolvedRoot: string; + canonicalRoot: string; +}; + +function isFileNotFoundError(error: unknown): boolean { + return ( + typeof error === "object" && + error !== null && + "code" in error && + (error as { code?: string }).code === "ENOENT" + ); +} + +function resolveRootPaths(root: string): RootPaths { + const resolvedRoot = path.resolve(root); + return { + resolvedRoot, + canonicalRoot: fs.realpathSync(resolvedRoot), + }; +} + +export function isPathWithinRoot(root: string, candidate: string): boolean { + const relative = path.relative(root, candidate); + return ( + relative !== ".." && + !relative.startsWith(`..${path.sep}`) && + !path.isAbsolute(relative) + ); +} + +export function normalizeRelativeDocumentPath( + name: unknown, +): string | undefined { + if (typeof name !== "string") { + return undefined; + } + const trimmed = name.trim(); + if ( + trimmed.length === 0 || + path.isAbsolute(trimmed) || + /^[a-zA-Z]:/.test(trimmed) + ) { + return undefined; + } + + const normalized = trimmed.replace(/\\/g, "/"); + const segments = normalized.split("/"); + if ( + segments.some( + (segment) => segment === "" || segment === "." || segment === "..", + ) + ) { + return undefined; + } + return segments.join("/"); +} + +export function resolveRealDirectory(absolutePath: string): string | undefined { + if (!path.isAbsolute(absolutePath)) { + return undefined; + } + try { + const canonicalPath = fs.realpathSync(absolutePath); + return fs.statSync(canonicalPath).isDirectory() + ? canonicalPath + : undefined; + } catch (error) { + if (isFileNotFoundError(error)) { + return undefined; + } + throw error; + } +} + +export function resolveExistingFileWithinRoot( + root: string, + requestedPath: string, +): string | undefined { + const rootPaths = resolveRootPaths(root); + const candidate = path.resolve(rootPaths.resolvedRoot, requestedPath); + if (!isPathWithinRoot(rootPaths.resolvedRoot, candidate)) { + return undefined; + } + try { + const canonicalFile = fs.realpathSync(candidate); + return isPathWithinRoot(rootPaths.canonicalRoot, canonicalFile) && + fs.statSync(canonicalFile).isFile() + ? canonicalFile + : undefined; + } catch (error) { + if (isFileNotFoundError(error)) { + return undefined; + } + throw error; + } +} + +function ensureDirectoryWithinRoot( + root: RootPaths, + relativeDirectory: string, +): string | undefined { + const candidate = path.resolve(root.resolvedRoot, relativeDirectory); + if (!isPathWithinRoot(root.resolvedRoot, candidate)) { + return undefined; + } + + const segments = path + .relative(root.resolvedRoot, candidate) + .split(path.sep) + .filter((segment) => segment.length > 0); + let currentDirectory = root.canonicalRoot; + for (const segment of segments) { + const nextDirectory = path.join(currentDirectory, segment); + let stats: fs.Stats | undefined; + try { + stats = fs.lstatSync(nextDirectory); + } catch (error) { + if (!isFileNotFoundError(error)) { + throw error; + } + } + + if (stats === undefined) { + fs.mkdirSync(nextDirectory); + } else if (stats.isSymbolicLink() || !stats.isDirectory()) { + return undefined; + } + + const canonicalDirectory = fs.realpathSync(nextDirectory); + if (!isPathWithinRoot(root.canonicalRoot, canonicalDirectory)) { + return undefined; + } + currentDirectory = canonicalDirectory; + } + return currentDirectory; +} + +export function resolveWritableFileWithinRoot( + root: string, + requestedPath: string, +): string | undefined { + const rootPaths = resolveRootPaths(root); + const candidate = path.resolve(rootPaths.resolvedRoot, requestedPath); + if (!isPathWithinRoot(rootPaths.resolvedRoot, candidate)) { + return undefined; + } + + const relativeParent = path.relative( + rootPaths.resolvedRoot, + path.dirname(candidate), + ); + const canonicalParent = ensureDirectoryWithinRoot( + rootPaths, + relativeParent, + ); + if (canonicalParent === undefined) { + return undefined; + } + + const writablePath = path.join(canonicalParent, path.basename(candidate)); + try { + const stats = fs.lstatSync(writablePath); + if (stats.isSymbolicLink() || !stats.isFile()) { + return undefined; + } + const canonicalFile = fs.realpathSync(writablePath); + return isPathWithinRoot(rootPaths.canonicalRoot, canonicalFile) + ? canonicalFile + : undefined; + } catch (error) { + if (isFileNotFoundError(error)) { + return writablePath; + } + throw error; + } +} diff --git a/ts/packages/agents/markdown/test/creationPathPolicy.spec.ts b/ts/packages/agents/markdown/test/creationPathPolicy.spec.ts new file mode 100644 index 0000000000..5872c5bc7e --- /dev/null +++ b/ts/packages/agents/markdown/test/creationPathPolicy.spec.ts @@ -0,0 +1,107 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { + normalizeRelativeDocumentPath, + resolveExistingFileWithinRoot, + resolveRealDirectory, + resolveWritableFileWithinRoot, +} from "../src/agent/pathPolicy.js"; + +describe("markdown creation path policy", () => { + let temporaryDirectory: string; + let root: string; + let sibling: string; + + beforeEach(() => { + temporaryDirectory = fs.mkdtempSync( + path.join(os.tmpdir(), "typeagent-markdown-create-path-"), + ); + root = path.join(temporaryDirectory, "Documents"); + sibling = path.join(temporaryDirectory, "Documents-backup"); + fs.mkdirSync(root); + fs.mkdirSync(sibling); + }); + + afterEach(() => { + fs.rmSync(temporaryDirectory, { recursive: true, force: true }); + }); + + test("resolves a new file and creates nested directories inside the root", () => { + expect(resolveWritableFileWithinRoot(root, "new.md")).toBe( + path.join(fs.realpathSync(root), "new.md"), + ); + expect(resolveWritableFileWithinRoot(root, "notes/nested/new.md")).toBe( + path.join(fs.realpathSync(root), "notes", "nested", "new.md"), + ); + expect( + fs.statSync(path.join(root, "notes", "nested")).isDirectory(), + ).toBe(true); + }); + + test("rejects paths outside the root", () => { + expect( + resolveWritableFileWithinRoot( + root, + path.join("..", "Documents-backup", "new.md"), + ), + ).toBeUndefined(); + }); + + test("rejects a symlink that resolves outside the root", () => { + const link = path.join(root, "linked"); + fs.symlinkSync(sibling, link, "junction"); + + expect( + resolveWritableFileWithinRoot(root, path.join("linked", "new.md")), + ).toBeUndefined(); + }); + + test("rejects a dangling link as a writable target", () => { + const link = path.join(root, "dangling"); + fs.symlinkSync(path.join(sibling, "missing"), link, "junction"); + + expect(resolveWritableFileWithinRoot(root, "dangling")).toBeUndefined(); + }); + + test("resolves only existing files inside the root", () => { + const document = path.join(root, "existing.md"); + fs.writeFileSync(document, "content"); + + expect(resolveExistingFileWithinRoot(root, "existing.md")).toBe( + fs.realpathSync(document), + ); + expect( + resolveExistingFileWithinRoot(root, "missing.md"), + ).toBeUndefined(); + expect( + resolveExistingFileWithinRoot( + root, + path.join("..", "Documents-backup", "outside.md"), + ), + ).toBeUndefined(); + }); + + test("normalizes relative document paths and rejects unsafe inputs", () => { + expect(normalizeRelativeDocumentPath(" notes\\nested\\plan ")).toBe( + "notes/nested/plan", + ); + expect(normalizeRelativeDocumentPath("../escape.md")).toBeUndefined(); + expect( + normalizeRelativeDocumentPath("sub/../escape.md"), + ).toBeUndefined(); + expect(normalizeRelativeDocumentPath("/tmp/escape.md")).toBeUndefined(); + expect(normalizeRelativeDocumentPath("C:escape.md")).toBeUndefined(); + }); + + test("accepts only existing absolute directories as workspace roots", () => { + expect(resolveRealDirectory(root)).toBe(fs.realpathSync(root)); + expect(resolveRealDirectory("relative")).toBeUndefined(); + expect( + resolveRealDirectory(path.join(root, "missing")), + ).toBeUndefined(); + }); +}); diff --git a/ts/packages/agents/markdown/test/markdownActionHandler.spec.ts b/ts/packages/agents/markdown/test/markdownActionHandler.spec.ts new file mode 100644 index 0000000000..e8af7a07d3 --- /dev/null +++ b/ts/packages/agents/markdown/test/markdownActionHandler.spec.ts @@ -0,0 +1,278 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { ActionContext, Storage } from "@typeagent/agent-sdk"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { instantiate } from "../src/agent/markdownActionHandler.js"; + +type TestAgentContext = { + currentDocument?: + | { + source: "session"; + storageKey: string; + } + | { + source: "workspace"; + filePath: string; + workspaceRoot: string; + }; + viewProcess?: { send: (message: unknown) => void } | undefined; + localHostPort: number; +}; + +describe("markdown document creation", () => { + let workspace: string; + + beforeEach(() => { + workspace = fs.mkdtempSync( + path.join(os.tmpdir(), "typeagent-markdown-create-"), + ); + }); + + afterEach(() => { + fs.rmSync(workspace, { recursive: true, force: true }); + }); + + function createContext(options?: { + workingDirectory?: string | undefined; + storage?: Storage | undefined; + viewProcess?: { send: (message: unknown) => void } | undefined; + }): { + context: ActionContext; + agentContext: TestAgentContext; + } { + const agentContext = { + localHostPort: 0, + viewProcess: options?.viewProcess, + }; + const workingDirectory = + options !== undefined && "workingDirectory" in options + ? options.workingDirectory + : workspace; + const context = { + workingDirectory, + sessionContext: { + agentContext, + sessionStorage: options?.storage, + }, + } as unknown as ActionContext; + return { context, agentContext }; + } + + test("creates a nested document with the requested content without model setup", async () => { + const savedModelSettings = Object.entries(process.env).filter( + ([key]) => + key.startsWith("AZURE_OPENAI_") || + key.startsWith("OPENAI_") || + key.startsWith("OLLAMA_") || + key === "MODEL_PROVIDER", + ); + for (const [key] of savedModelSettings) { + delete process.env[key]; + } + const viewMessages: unknown[] = []; + const { context, agentContext } = createContext({ + viewProcess: { + send: (message) => { + viewMessages.push(message); + }, + }, + }); + + try { + const result = await instantiate().executeAction!( + { + schemaName: "markdown", + actionName: "createDocument", + parameters: { + name: "notes/nested/plan", + content: "# Plan\n\nInitial content.", + }, + }, + context, + ); + if (result === undefined || "error" in result) { + throw new Error("Expected successful document creation"); + } + + const expectedPath = path.join( + fs.realpathSync(workspace), + "notes", + "nested", + "plan.md", + ); + expect(fs.readFileSync(expectedPath, "utf-8")).toBe( + "# Plan\n\nInitial content.", + ); + expect(agentContext).toMatchObject({ + currentDocument: { + source: "workspace", + filePath: expectedPath, + workspaceRoot: fs.realpathSync(workspace), + }, + }); + expect(result.tokenUsage).toEqual({ + prompt_tokens: 0, + completion_tokens: 0, + total_tokens: 0, + }); + expect(result.activityContext?.openLocalView).toBe(false); + expect(viewMessages).toHaveLength(0); + } finally { + for (const [key, value] of savedModelSettings) { + process.env[key] = value; + } + } + }); + + test.each([ + ["traversal", "../escape"], + ["nested traversal", "notes/../../escape"], + ["absolute", path.resolve("escape")], + ["drive-qualified", "C:escape"], + ])("rejects %s paths", async (_label, name) => { + const { context } = createContext(); + await expect( + instantiate().executeAction!( + { + schemaName: "markdown", + actionName: "createDocument", + parameters: { name }, + }, + context, + ), + ).rejects.toThrow(/safe relative path/); + }); + + test("rejects a nested symlink escape", async () => { + const outside = fs.mkdtempSync( + path.join(os.tmpdir(), "typeagent-markdown-outside-"), + ); + fs.symlinkSync(outside, path.join(workspace, "linked"), "junction"); + const { context } = createContext(); + + try { + await expect( + instantiate().executeAction!( + { + schemaName: "markdown", + actionName: "createDocument", + parameters: { + name: "linked/escape", + content: "must stay inside", + }, + }, + context, + ), + ).rejects.toThrow(/not writable within the working directory/); + expect(fs.existsSync(path.join(outside, "escape.md"))).toBe(false); + } finally { + fs.rmSync(outside, { recursive: true, force: true }); + } + }); + + test("falls back to session storage without a working directory", async () => { + const files = new Map(); + const storage = { + exists: async (name: string) => files.has(name), + read: async (name: string) => files.get(name) ?? "", + write: async (name: string, content: string) => { + files.set(name, content); + }, + } as unknown as Storage; + const { context, agentContext } = createContext({ + workingDirectory: undefined, + storage, + }); + + const result = await instantiate().executeAction!( + { + schemaName: "markdown", + actionName: "createDocument", + parameters: { + name: "notes", + content: "# Stored note", + }, + }, + context, + ); + if (result === undefined || "error" in result) { + throw new Error("Expected successful document creation"); + } + + expect(files.get("notes.md")).toBe("# Stored note"); + expect(agentContext).toMatchObject({ + currentDocument: { + source: "session", + storageKey: "notes.md", + }, + }); + expect(result.activityContext?.openLocalView).toBe(true); + }); + + test("opens an existing workspace document without opening the session-rooted view", async () => { + const documentPath = path.join(workspace, "notes.md"); + fs.writeFileSync(documentPath, "# Existing"); + const viewMessages: unknown[] = []; + const { context, agentContext } = createContext({ + viewProcess: { + send: (message) => { + viewMessages.push(message); + }, + }, + }); + + const result = await instantiate().executeAction!( + { + schemaName: "markdown", + actionName: "openDocument", + parameters: { name: "notes" }, + }, + context, + ); + if (result === undefined || "error" in result) { + throw new Error("Expected successful document open"); + } + + expect(agentContext.currentDocument).toEqual({ + source: "workspace", + filePath: fs.realpathSync(documentPath), + workspaceRoot: fs.realpathSync(workspace), + }); + expect(result.activityContext?.openLocalView).toBe(false); + expect(viewMessages).toHaveLength(0); + }); + + test("does not create a missing document when opening it", async () => { + const { context } = createContext(); + + await expect( + instantiate().executeAction!( + { + schemaName: "markdown", + actionName: "openDocument", + parameters: { name: "missing" }, + }, + context, + ), + ).rejects.toThrow(/does not exist within the working directory/); + expect(fs.existsSync(path.join(workspace, "missing.md"))).toBe(false); + }); + + test("requires a working directory or session storage", async () => { + const { context } = createContext({ workingDirectory: undefined }); + + await expect( + instantiate().executeAction!( + { + schemaName: "markdown", + actionName: "createDocument", + parameters: { name: "notes" }, + }, + context, + ), + ).rejects.toThrow(/working directory or session storage/); + }); +}); diff --git a/ts/packages/agents/markdown/test/tsconfig.json b/ts/packages/agents/markdown/test/tsconfig.json index fb7bb74fdd..7aa38d62cf 100644 --- a/ts/packages/agents/markdown/test/tsconfig.json +++ b/ts/packages/agents/markdown/test/tsconfig.json @@ -7,5 +7,5 @@ "types": ["node", "jest"] }, "include": ["./**/*"], - "references": [{ "path": "../src/view/route" }] + "references": [{ "path": "../src/agent" }, { "path": "../src/view/route" }] } diff --git a/ts/packages/cli/src/commands/run/request.ts b/ts/packages/cli/src/commands/run/request.ts index 139e9eaf58..f45adefb99 100644 --- a/ts/packages/cli/src/commands/run/request.ts +++ b/ts/packages/cli/src/commands/run/request.ts @@ -95,6 +95,7 @@ export default class RequestCommand extends Command { conversation.dispatcher, `@dispatcher request ${args.request}`, this.loadAttachment(args.attachment), + { workingDirectory: process.cwd() }, ); }); } finally { diff --git a/ts/packages/dispatcher/dispatcher/src/execute/actionContext.ts b/ts/packages/dispatcher/dispatcher/src/execute/actionContext.ts index e368a4b028..1182fbabb3 100644 --- a/ts/packages/dispatcher/dispatcher/src/execute/actionContext.ts +++ b/ts/packages/dispatcher/dispatcher/src/execute/actionContext.ts @@ -78,6 +78,7 @@ export function getActionContext( const actionContext: ActionContext = { streamingContext: undefined, isFromReasoningLoop: context.isInsideReasoningLoop, + workingDirectory: systemContext.currentOptions?.workingDirectory, activityContext: // Only make activityContext available if the action is from the same agent. context.activityContext?.appAgentName === appAgentName diff --git a/ts/packages/dispatcher/dispatcher/test/actionContext.spec.ts b/ts/packages/dispatcher/dispatcher/test/actionContext.spec.ts index c0c1aa73c4..6b5f977a92 100644 --- a/ts/packages/dispatcher/dispatcher/test/actionContext.spec.ts +++ b/ts/packages/dispatcher/dispatcher/test/actionContext.spec.ts @@ -5,12 +5,14 @@ import { getActionContext } from "../src/execute/actionContext.js"; // Builds the minimal CommandHandlerContext surface that getActionContext and // makeClientIOMessage touch for non-error display content. -function makeContext() { +function makeContext(workingDirectory?: string) { const calls: { type: string; mode?: string }[] = []; const context = { displayCount: 0, reasoningSourceIcon: undefined, collectCommandResult: false, + currentOptions: + workingDirectory === undefined ? undefined : { workingDirectory }, metricsManager: undefined, agents: { getSessionContext: () => ({}) as any, @@ -79,4 +81,17 @@ describe("getActionContext displayCount tracking", () => { actionContext.actionIO.appendDisplay(content, "temporary"); expect(context.displayCount).toBe(0); }); + + it("exposes the host-authorized working directory", () => { + const workingDirectory = "C:\\workspace"; + const { context } = makeContext(workingDirectory); + const { actionContext } = getActionContext( + "agent", + context, + requestId, + 0, + ); + + expect(actionContext.workingDirectory).toBe(workingDirectory); + }); });