Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 86 additions & 21 deletions src/integrations/misc/open-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,39 +28,104 @@ interface OpenFileOptions {

export async function openFile(filePath: string, options: OpenFileOptions = {}) {
try {
// Get workspace root
const workspaceRoot = getWorkspacePath()
const homeDir = os.homedir()
const originalFilePathForError = filePath // Keep original for error messages

// If path starts with ./, resolve it relative to workspace root if available
// Otherwise, use the path as provided without modification
const fullPath = filePath.startsWith("./")
? workspaceRoot
? path.join(workspaceRoot, filePath.slice(2))
: filePath
: filePath
const attemptPaths: string[] = []

const uri = vscode.Uri.file(fullPath)
if (filePath.startsWith("./")) {
const relativePart = filePath.slice(2)
Copy link
Member

@daniel-lxs daniel-lxs May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @KJ7LNW Would it be a good idea to prevent directory traversal?
Something like this could work:

if (relativePart.includes('..')) {
    throw new Error('Path contains directory traversal pattern');
}

Or maybe something more sophisticated would make sense.

I just tested your implementation and I see some weird behavior when the model outputs a path like ..

Copy link
Contributor Author

@KJ7LNW KJ7LNW May 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did the directory that it referenced using ../ exist, or was it just missing?

Can you provide a specific example so I can understand better?

I think we need to expect the model will sometimes render things in a way that is not expected; worst case the link is broken, but I do not think that is a problem.

Render-output handling is always best effort: the models are pretty good, but you never know what you are going to get.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's how it behaves for me, it's probably a non-issue though, that's why I left this PR for review.

2025-05-29.17-17-13.mp4

Apologies for the awful quality.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is strange behavior. if anything, .. should mean ${workspace}/.. but you're right, non-issue no one will need to click on ".."

if (workspaceRoot) {
attemptPaths.push(path.join(workspaceRoot, relativePart))
}
if (homeDir) {
const homePath = path.join(homeDir, relativePart)
// Add home path if it's different from what might have been added via workspaceRoot
// (e.g. if workspaceRoot itself is the home directory)
if (!attemptPaths.includes(homePath)) {
attemptPaths.push(homePath)
}
}
// If no workspace and no home, or if paths were identical.
if (attemptPaths.length === 0) {
attemptPaths.push(filePath) // Try the original relative path as a last resort
}
} else {
attemptPaths.push(filePath) // Assumed absolute or directly resolvable
}

// Check if file exists
try {
await vscode.workspace.fs.stat(uri)
} catch {
// File doesn't exist
if (!options.create) {
throw new Error("File does not exist")
let fileStat: vscode.FileStat | undefined
let successfulUri: vscode.Uri | undefined

for (const p of attemptPaths) {
try {
const tempUri = vscode.Uri.file(p)
fileStat = await vscode.workspace.fs.stat(tempUri)
successfulUri = tempUri // Path found
break // Exit loop once a path is successfully stated
} catch (e) {
// Stat failed for this path, continue to the next one
}
}

// Create with provided content or empty string
const content = options.content || ""
await vscode.workspace.fs.writeFile(uri, Buffer.from(content, "utf8"))
let uriToProcess: vscode.Uri

if (fileStat && successfulUri) {
// Path was found
if (fileStat.type === vscode.FileType.Directory) {
await vscode.commands.executeCommand("revealInExplorer", successfulUri)
// Attempt to expand the revealed directory in the explorer.
// This requires the explorer to have focus and the item to be selected.
// A slight delay might sometimes be needed for focus to shift,
// but often it works immediately after revealInExplorer.
try {
await vscode.commands.executeCommand("list.expand")
} catch (expandError) {
// Log or handle if expansion specifically fails, though often not critical
console.warn("Could not expand directory in explorer:", expandError)
}
return // Done for directories
}
uriToProcess = successfulUri // It's an existing file
} else {
// Path was not found in any attempted locations. Consider creation.
if (
options.create &&
!(originalFilePathForError.endsWith("/") || originalFilePathForError.endsWith("\\"))
) {
let pathToCreateAt: string
if (originalFilePathForError.startsWith("./")) {
const relativePart = originalFilePathForError.slice(2)
if (workspaceRoot) {
pathToCreateAt = path.join(workspaceRoot, relativePart)
} else if (homeDir) {
pathToCreateAt = path.join(homeDir, relativePart)
} else {
pathToCreateAt = originalFilePathForError // Fallback: use original relative path
}
} else {
pathToCreateAt = originalFilePathForError // Absolute path
}

uriToProcess = vscode.Uri.file(pathToCreateAt)
const contentToCreate = options.content || ""
await vscode.workspace.fs.writeFile(uriToProcess, Buffer.from(contentToCreate, "utf8"))
// File is now created, uriToProcess points to it.
} else {
// Not creating, or it's a directory-like path that doesn't exist.
throw new Error(`Path does not exist: ${originalFilePathForError}`)
}
}

// At this point, uriToProcess points to an existing file or a newly created file.
// Check if the document is already open in a tab group that's not in the active editor's column
try {
for (const group of vscode.window.tabGroups.all) {
const existingTab = group.tabs.find(
(tab) =>
tab.input instanceof vscode.TabInputText && arePathsEqual(tab.input.uri.fsPath, uri.fsPath),
tab.input instanceof vscode.TabInputText &&
arePathsEqual(tab.input.uri.fsPath, uriToProcess.fsPath),
)
if (existingTab) {
const activeColumn = vscode.window.activeTextEditor?.viewColumn
Expand All @@ -75,7 +140,7 @@ export async function openFile(filePath: string, options: OpenFileOptions = {})
}
} catch {} // not essential, sometimes tab operations fail

const document = await vscode.workspace.openTextDocument(uri)
const document = await vscode.workspace.openTextDocument(uriToProcess)
const selection =
options.line !== undefined ? new vscode.Selection(options.line - 1, 0, options.line - 1, 0) : undefined
await vscode.window.showTextDocument(document, {
Expand Down
Loading