Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

macOS: TypeScript installs recursive file watchers on ~/Library leading to access prompts #59831

Closed
bpasero opened this issue Sep 2, 2024 · 13 comments · Fixed by #59869
Closed
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@bpasero
Copy link
Member

bpasero commented Sep 2, 2024

🔎 Search Terms

file watcher

🕗 Version & Regression Information

  • This changed between versions 5.5 and 5.4 with the introduction of TS using VS Code for file watching.

🙁 Actual behavior

We have seen a user in microsoft/vscode#208105 report that TypeScript installed a recursive file watcher on ~/Library. This can trigger permission dialogs on macOS starting from 14.x.

image

2024-09-01 16:19:25.676 [trace] MainThreadFileSystemEventService#$watch(): 
request to start watching correlated (extension: vscode.typescript-language-features, 
path: file:///Users/tmm1/Library, recursive: true, session: 0.7154528361377308)

🙂 Expected behavior

TypeScript should not install recursive file watchers so broadly.

//cc @DanielRosenwasser @sheetalkamat maybe you could comment how such a path can be used for watching? Are there global installations of TypeScript in ~/Library?

@tmm1
Copy link

tmm1 commented Sep 2, 2024

Here is a quick and dirty patch:

diff --git a/extensions/typescript-language-features/src/tsServer/fileWatchingManager.ts b/extensions/typescript-language-features/src/tsServer/fileWatchingManager.ts
index 27b9ec3d7f6c..d9a4e02ee7db 100644
--- a/extensions/typescript-language-features/src/tsServer/fileWatchingManager.ts
+++ b/extensions/typescript-language-features/src/tsServer/fileWatchingManager.ts
@@ -60,6 +60,12 @@ export class FileWatcherManager implements IDisposable {
 		if (watchParentDirs && uri.scheme !== Schemes.untitled) {
 			// We need to watch the parent directories too for when these are deleted / created
 			for (let dirUri = Utils.dirname(uri); dirUri.path.length > 1; dirUri = Utils.dirname(dirUri)) {
+				// on macOS, avoid turning `~/Library/Caches/typescript/5.5/...` into a watcher on `~/Library`
+				// this prevents security alerts, due to a parcel-bundler/watcher bug trying to open(~/Library/Containers)
+				// https://github.com/microsoft/vscode/issues/208105#issuecomment-2323462416
+				if (dirUri.path.endsWith('/Library/Caches')) {
+					break;
+				}
 				const dirWatcher: DirWatcherEntry = { uri: dirUri, listeners: [] };
 
 				let parentDirWatcher = this._dirWatchers.get(dirUri);

@tmm1
Copy link

tmm1 commented Sep 3, 2024

it's not clear to me why the extension is trying to traverse upward and install additional watcher up through the root?

perhaps the proper fix is:

--- a/extensions/typescript-language-features/src/tsServer/serverProcess.browser.ts
+++ b/extensions/typescript-language-features/src/tsServer/serverProcess.browser.ts
@@ -115,7 +115,7 @@ class WorkerServerProcess implements TsServerProcess {
                                }
                                case 'watchDirectory':
                                case 'watchFile': {
-                                       this._watches.create(event.data.id, vscode.Uri.from(event.data.uri), /*watchParentDirs*/ true, !!event.data.recursive, {
+                                       this._watches.create(event.data.id, vscode.Uri.from(event.data.uri), /*watchParentDirs*/ false, !!event.data.recursive, {
                                                change: uri => this._watcher.postMessage({ type: 'watch', event: 'change', uri }),
                                                create: uri => this._watcher.postMessage({ type: 'watch', event: 'create', uri }),

@tmm1
Copy link

tmm1 commented Sep 3, 2024

I think I understand.. the intention of the watchParentDirs flag is to install a non-recursive set of watchers up the directory tree, however on macOS each of these is recursive by default and thus ends up sending events for unnecessary files.

Unfortunately it doesn't appear that vscode.workspace.createFileSystemWatcher provides control over the recursive option?

For the initially requested directory, FileWatcherManager.create appends ** to request recursion..

const watcher = vscode.workspace.createFileSystemWatcher(new vscode.RelativePattern(uri, isRecursive ? '**' : '*'), !listeners.create, !listeners.change, !listeners.delete);

@tmm1
Copy link

tmm1 commented Sep 3, 2024

Never mind, the issue is not related to watchParentDirs.

I found in tsserver.log that it is explicitly requesting a resursive watcher on ~/Library, because it infers the default cache location of ~/Library/Caches/typescript/5.5/node_modules is rooted there.

EDIT: It appears I may be using another typescript extension that's causing this issue

@tmm1
Copy link

tmm1 commented Sep 3, 2024

I believe this is where a directory watcher on ~/Library is installed:

// path in global cache, watch global cache
if (containsPath(this.projectService.typingsInstaller.globalTypingsCacheLocation!, file, this.currentDirectory, !this.useCaseSensitiveFileNames())) {
createProjectWatcher(this.projectService.typingsInstaller.globalTypingsCacheLocation!, TypingWatcherType.DirectoryWatcher);
continue;

@sheetalkamat
Copy link
Member

@tmm1 we need a tsserver log for this issue so we can determine what is the best way to fix this.

Thanks

@tmm1
Copy link

tmm1 commented Sep 3, 2024

Okay. This is hard to reproduce as it happens very intermittently (not sure when tsserver decides to use ~/Library/Caches/typescript but it's not all the time).

I will remember to capture and share the logs next time.

@tmm1
Copy link

tmm1 commented Sep 3, 2024

Here is what I see when things go haywire:

Info 12165[16:26:43.910] Starting updateGraphWorker: Project: /dev/null/inferredProject1*
Info 12167[16:26:43.924] DirectoryWatcher:: Added:: WatchInfo: /Users/tmm1/Library 1 undefined Project: /dev/null/inferredProject1*
 WatchType: Failed Lookup Locations
Info 12168[16:26:43.924] event:
    {"seq":0,"type":"event","event":"createDirectoryWatcher","body":{"id":5825,"path":"/Users/tmm1/Library","recursive":true,"ignoreUpdate":true}}
Info 12169[16:26:43.924] Elapsed:: 0.08395910263061523ms DirectoryWatcher:: Added:: WatchInfo: /Users/tmm1/Library 1 undefined Project: /dev/null/inferredProject1* WatchType: Failed Lookup Locations
Info 12170[16:26:43.924] FileWatcher:: Added:: WatchInfo: /Users/tmm1/Library/Caches/typescript/5.5/node_modules/undici-types/package.json 2000 undefined Project: /dev/null/inferredProject1* WatchType: File location affecting resolution
Info 12171[16:26:43.924] event:
    {"seq":0,"type":"event","event":"createFileWatcher","body":{"id":5826,"path":"/Users/tmm1/Library/Caches/typescript/5.5/node_modules/undici-types/package.json"}}
Info 12172[16:26:43.937] FileWatcher:: Added:: WatchInfo: /Users/tmm1/Library/Caches/typescript/5.5/node_modules/@types/node/package.json 2000 undefined Project: /dev/null/inferredProject1* WatchType: File location affecting resolution
Info 12173[16:26:43.937] event:
    {"seq":0,"type":"event","event":"createFileWatcher","body":{"id":5827,"path":"/Users/tmm1/Library/Caches/typescript/5.5/node_modules/@types/node/package.json"}}
Info 12174[16:26:44.078] Finishing updateGraphWorker: Project: /dev/null/inferredProject1* projectStateVersion: 3 projectProgramVersion: 2 structureChanged: true structureIsReused:: SafeModules Elapsed: 168.39091682434082ms
Info 12175[16:26:44.078] Project '/dev/null/inferredProject1*' (Inferred)
Info 12176[16:26:44.080]        Files (143)

@sheetalkamat
Copy link
Member

I need the complete log pls. You can replace all sensitive information in the log but file list is important to see whats going on

@tmm1
Copy link

tmm1 commented Sep 3, 2024

tsserverbug.log

@sheetalkamat sheetalkamat added the Needs Investigation This issue needs a team member to investigate its status. label Sep 3, 2024
@sheetalkamat sheetalkamat self-assigned this Sep 3, 2024
@sheetalkamat
Copy link
Member

sheetalkamat commented Sep 4, 2024

From the log it seems like you have plugin enabled. Can you please disable the plugins and then generate new tsserver log file and see if this still repros. I am still trying to find why "Library" is watched instead of "node_modules" in it and have no repro yet

Edit1: I think for some reason your current directory sent to tsserver is "/" which is what probably causing this . will need to really see if i can repro it that way. But may be your plugin is causing that to happen?

Edit2: Yes the repro needs the current directory to be "/" and then i am able to see the issue.

@tmm1
Copy link

tmm1 commented Sep 4, 2024

I am not able to repro easily without the plugin.

I believe what's happening is that the plugin uses in-memory documents with no real file path. These are causing confusion in resolving project root and resulting in file watchers on strangely rooted directories.

cc #59508

@sheetalkamat
Copy link
Member

Yeah but its not issue when its using correct location of vscode which is normally a path thats not "/" and we will watch "node_modules" instead of determining what to watch based off of "root directory". Plugin is what is causing current directory for tsserver to be "/" and not something like "/Applications/Visual Studio Code.app/Contents/Resources/app/bin"

You can create a document that's not on disk by:
Open code (without "." or some folder path)
Close any existing folder/workspaces
create new file, set type to javascript and you should be able to get exact same behavior on opening a file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
3 participants