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

“Visual Studio Code” would like to access data from other apps. #208105

Closed
thydadduong opened this issue Mar 19, 2024 · 22 comments
Closed

“Visual Studio Code” would like to access data from other apps. #208105

thydadduong opened this issue Mar 19, 2024 · 22 comments
Assignees
Labels
author-verification-requested Issues potentially verifiable by issue author bug Issue identified by VS Code Team member as probable bug file-watcher File watcher macos Issues with VS Code on MAC/OS X verified Verification succeeded

Comments

@thydadduong
Copy link

Type: Bug

this popup keep show up althought I click allow multiple time

VS Code version: Code 1.87.2 (863d258, 2024-03-08T15:21:31.043Z)
OS version: Darwin x64 23.4.0
Modes:

System Info
Item Value
CPUs Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz (16 x 2300)
GPU Status 2d_canvas: enabled
canvas_oop_rasterization: disabled_off
direct_rendering_display_compositor: disabled_off_ok
gpu_compositing: enabled
multiple_raster_threads: enabled_on
opengl: enabled_on
rasterization: enabled
raw_draw: disabled_off_ok
skia_graphite: disabled_off
video_decode: enabled
video_encode: enabled
webgl: enabled
webgl2: enabled
webgpu: enabled
Load (avg) 3, 4, 9
Memory (System) 16.00GB (0.05GB free)
Process Argv --crash-reporter-id bfcc286a-8546-463f-8ff2-2dcb413b0491 --crash-reporter-id bfcc286a-8546-463f-8ff2-2dcb413b0491
Screen Reader no
VM 0%
Extensions (60)
Extension Author (truncated) Version
vscode-sql-formatter adp 1.4.4
vscode-nginx-conf ahm 0.3.2
project-colorize Ang 1.0.3
ng-template Ang 17.3.0
quitcontrol-vscode art 4.0.0
vscode-tailwindcss bra 0.10.5
dart-code Dar 3.84.0
flutter Dar 3.84.0
gitlens eam 14.9.0
vscode-html-css ecm 2.0.9
vscode-author-generator edw 0.2.2
prettier-vscode esb 10.1.0
auto-close-tag for 0.5.15
auto-rename-tag for 0.1.10
shell-format fox 7.2.5
vscode-google-translate fun 1.4.13
codespaces Git 1.16.14
copilot Git 1.175.0
copilot-chat Git 0.13.1
copilot-labs Git 0.17.1121
live-sass gle 6.1.2
todo-tree Gru 0.0.226
vscode-drawio hed 1.6.6
vscode-edit-csv jan 0.9.1
vsc-python-indent Kev 1.18.0
rainbow-csv mec 3.11.0
dotenv mik 1.0.1
vscode-docker ms- 1.29.0
debugpy ms- 2024.2.0
python ms- 2024.2.1
jupyter ms- 2024.2.0
jupyter-keymap ms- 1.1.2
jupyter-renderers ms- 1.0.17
vscode-jupyter-cell-tags ms- 0.1.8
vscode-jupyter-slideshow ms- 0.1.5
remote-containers ms- 0.348.0
remote-ssh ms- 0.109.0
remote-ssh-edit ms- 0.86.0
remote-explorer ms- 0.4.3
remote-server ms- 1.5.1
autodocstring njp 0.6.1
indent-rainbow ode 8.3.1
parallels-desktop Par 1.0.12
postman-for-vscode Pos 0.19.1
vscode-thunder-client ran 2.19.6
LiveServer rit 5.7.9
svg-preview Sim 2.8.3
code-spell-checker str 3.0.1
pdf tom 1.2.2
vscode-input-sequence tom 0.2.0
vscodeintellicode Vis 1.3.1
rainbow-tags vol 0.4.1
vscode-gradle vsc 3.13.5
vscode-icons vsc 12.7.0
volar Vue 2.0.6
vuetify-vscode vue 0.2.0
vscode-todo-highlight way 1.0.5
vscode-nginx wil 0.7.2
fontview yiw 1.0.1
html-css-class-completion Zig 1.20.0

(1 theme extensions excluded)

A/B Experiments
vsliv368cf:30146710
vspor879:30202332
vspor708:30202333
vspor363:30204092
vscoreces:30445986
vscod805cf:30301675
binariesv615:30325510
vsaa593cf:30376535
py29gd2263:30899288
c4g48928:30535728
azure-dev_surveyone:30548225
a9j8j154:30646983
962ge761:30959799
pythongtdpath:30769146
welcomedialog:30910333
pythonidxpt:30866567
pythonnoceb:30805159
asynctok:30898717
pythontestfixt:30902429
pythonregdiag2:30936856
pyreplss1:30897532
pythonmypyd1:30879173
pythoncet0:30885854
2e7ec940:30992800
pythontbext0:30879054
accentitlementsc:30887149
dsvsc016:30899300
dsvsc017:30899301
dsvsc018:30899302
cppperfcontrol:30979541
d34g3935:30971562
fegfb526:30981948
bg6jg535:30979843
ccp2r3:30993541
dsvsc020:30976470
pythonait:30992953

@noelleleigh
Copy link

Possible duplicate of #201087

@whitezo
Copy link

whitezo commented Jun 17, 2024

Still happening on Version: 1.90.1 despite the other ticket being closed.

@JimW
Copy link

JimW commented Jul 20, 2024

This dialog box needs an explanation as 35 others also agreed within a similar closed issue. It shows a lack of respect for people's right to manage their security. To blindly click allow is not very smart..

@julienreszka
Copy link

They locked the issue #201087 with no explanation whatsoever. I wonder why

@rmorabia
Copy link

This is a duplicate of #201087, but there was no solution given or official response from microsoft about why this is happening and how to set a setting to stop it from asking every single time. This has been non-stop for about 6 months.

@tmm1
Copy link
Contributor

tmm1 commented Sep 1, 2024

I managed to catch this in a debugger:

(llnode) up
frame #1: 0x0000000188599de8 libsystem_kernel.dylib`open + 64
libsystem_kernel.dylib`open:
->  0x188599de8 <+64>: mov    x21, x0
    0x188599dec <+68>: tbnz   w0, #0x1f, 0x188599e58    ; <+176>
    0x188599df0 <+72>: adrp   x8, 420761
    0x188599df4 <+76>: ldr    x8, [x8, #0xd58]
(llnode) p (char*)$x20
(char *) 0x0000000100bb0090 "/Users/tmm1/Library/Containers/com.apple.news.widget/Data/Library/Caches/com.apple.news.public-production-143441"
(llnode) up
frame #2: 0x0000000100c54404 watcher.node`FSEventsCallback(__FSEventStream const*, void*, unsigned long, void*, unsigned int const*, unsigned long long const*) + 812

The issue appears to occur when a --type=utility --utility-sub-type=node.mojom.NodeService process calls open() on recently changed files, which often include cache files associated with other apps.

The implicated code-path is here:

https://github.com/parcel-bundler/watcher/blob/99cf43b739b25bbd3538506dbc4e87a202f7af84/src/macos/FSEventsBackend.cc#L21-L25

// macOS has a case insensitive file system by default. In order to detect
// file renames that only affect case, we need to get the canonical path
// and compare it with the input path to determine if a file was created or deleted.
bool pathExists(char *path) {
  int fd = open(path, O_RDONLY | O_SYMLINK);

https://github.com/parcel-bundler/watcher/blob/99cf43b739b25bbd3538506dbc4e87a202f7af84/src/macos/FSEventsBackend.cc#L137-L141

      // If multiple flags were set, then we need to call `stat` to determine if the file really exists.
      // This helps disambiguate creates, updates, and deletes.
      struct stat file;
      if (!pathExists(paths[i]) || stat(paths[i], &file)) {
        // File does not exist, so we have to assume it was removed. This is not exact since the

And was introduced in https://github.com/parcel-bundler/watcher/pull/69/files#diff-6202fd99b7ea6bfc7a55c32b204393c7545302a980c15058a43b4fc1fade4747R111

@tmm1
Copy link
Contributor

tmm1 commented Sep 1, 2024

I found this in my window2/renderer.1.log:

2024-09-01 16:19:25.663 [trace] MainThreadFileSystemEventService#$watch(): request to start watching correlated (extension: vscode.typescript-language-features, path: file:///Users/tmm1/Library/Caches/typescript/5.5/node_modules/@types/node/package.json, recursive: false, session: 0.7153965884819184)

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)

@bpasero
Copy link
Member

bpasero commented Sep 2, 2024

@tmm1 great finding, as a mitigation we could also think about forcing that folder ~/Library/Containers as a exclude pattern when using parcel watcher:

ignore: watcher.request.excludes

@deepak1556
Copy link
Collaborator

@tmm1 thanks for investigating this issue, this is great find! I believe this privacy prompt should only show when VSCode attempts to access data from an app sandbox container (refs https://developer.apple.com/videos/play/wwdc2023/10053) which would be in ~/Library/Containers.

@bpasero from the logs posted in #208105 (comment), seems like the typescript extension is setting up a recursive watch inside ~/Library possibly because of global typescript installation. I think we should restrict watching ~/Library/Containers as a universal blocklist in vscode.

@bpasero
Copy link
Member

bpasero commented Sep 2, 2024

I have opened microsoft/TypeScript#59831 on TypeScript side and will push a change to hardcode ~/Library/Containers as ignore option for parcel watcher.

If anyone knows more paths like these that result in access dialogs that are not persisted per application (and thus very annoying), please let me know!

@bpasero bpasero closed this as completed in 6e8e175 Sep 2, 2024
@vs-code-engineering vs-code-engineering bot added the unreleased Patch has not yet been released in VS Code Insiders label Sep 2, 2024
@tmm1
Copy link
Contributor

tmm1 commented Sep 2, 2024

Thanks @bpasero! Though, I do wonder if your commit will help mitigate this.. the issue is that a watcher is installed onto ~/Library (which then triggers open() on changed files in Containers). But you've added an ignore for ~/Library/Containers which is never watched directly. It may make sense to add ~/Library itself to the ignore list?

@deepak1556
Copy link
Collaborator

deepak1556 commented Sep 2, 2024

open on Containers path is triggered because of recursive watchers set on ~/Library, we are just removing any watchers that will get set for ~/Library/Containers via the exclude rule when this happens. We cannot unset watcher for ~/Library since the typescript extension still needs it for its own watching of the global installed files.

@tmm1
Copy link
Contributor

tmm1 commented Sep 3, 2024

I'm pretty sure a watcher for ~/Library/Containers is never requested, so an exclude won't help.

What I observed was:

  • add watcher on ~/Library with recursive=true
  • get event on ~/Library/Containers/com.apple.news.widget/Data/Library/Caches/com.apple.news.public-production-143441
  • try to open said file and trigger error

The macOS FSEvents api is recursive by default, so adding a recursive watcher on ~/Library does not result in another watcher or open on ~/Library/Container

The issue happens later, when the recursive watch notices files changing inside ~/Library. I see open calls to all sorts of random files such as:

  • ~/Library/Safari/RecentlyClosedTabs.plist
  • ~/Library/Containers/com.tinyspeck.slackmacgap/Data/Library/Application Support/Slack/TransportSecurity

any file being modified under Library is opened by parcel-build/watcher


EDIT: trace logs from Output > File Watcher

2024-09-02 17:42:42.900 [trace] 

[File Watcher] request stats:

[Summary]
- Recursive Requests:     total: 38, suspended: 14, polling: 6
- Non-Recursive Requests: total: 327, suspended: 0, polling: 0
- Recursive Watchers:     total: 16, active: 16, failed: 0, stopped: 0
- Non-Recursive Watchers: total: 327, active: 19, failed: 3, reusing: 305
- I/O Handles Impact:     total: 41

[Recursive Requests (38, suspended: 14, polling: 6)]:
 /Users/tmm1/Library   (excludes: <none>, includes: <all>, filter: [Added, Deleted], correlationId: 336)
...

@deepak1556
Copy link
Collaborator

deepak1556 commented Sep 3, 2024

@tmm1 this is what I tried,

// watcher.js

const watcher = require('@parcel/watcher');
const path = require('path');
const { homedir } = require('os');

async function watch() {
    // Subscribe to events
    let subscription = await watcher.subscribe('/Users/demohan/Library', async (err, events) => {
        for (let event of events) {
            if (event.path.indexOf('Containers') !== -1) {
                console.log(event);
            }
        }
    }, {
        ignore: [
            path.join(homedir(), 'Library', 'Containers')
        ]
    });
}

watch();
  • Open terminal.app
  • Run above file node watcher.js
  • Open any macOS sandboxed app, tested with weathers.app
  • Privacy prompt is not seen
  • Close terminal.app, not just the active session so that active tcc rule is invalidated for the application.

Remove the ignore option from above script and re-run the steps. You can see the privacy prompt show up this time. Let me know if something is missing in this test case.

@tmm1
Copy link
Contributor

tmm1 commented Sep 3, 2024

I realized the intention of the code is to install a non-recursive watch on ~/Library, but its ending up recursive. See microsoft/TypeScript#59831 (comment)

I believe this is a bug introduced in https://github.com/microsoft/vscode/pull/139881/files#diff-a6ce3fa0fba995f3aeda03efa26f5e51cbab5e424cdf99869120da5539ee0a88R103-R106 where any watcher with a / is considered recursive.

--- a/src/vs/workbench/api/common/extHostFileSystemEventService.ts
+++ b/src/vs/workbench/api/common/extHostFileSystemEventService.ts
@@ -129,7 +129,7 @@ class FileSystemWatcher implements vscode.FileSystemWatcher {
                const proxy = mainContext.getProxy(MainContext.MainThreadFileSystemEventService);

                let recursive = false;
-               if (globPattern.pattern.includes(GLOBSTAR) || globPattern.pattern.includes(GLOB_SPLIT)) {
+               if (globPattern.pattern.includes(GLOBSTAR)) {
                        recursive = true; // only watch recursively if pattern indicates the need for it
                }

@tmm1
Copy link
Contributor

tmm1 commented Sep 3, 2024

Remove the ignore option from above script and re-run the steps. You can see the privacy prompt show up this time. Let me know if something is missing in this test case.

Your test case is sound. Thank you for sharing it.

I misunderstood how parcel's ignore option works, and now see how PREDEFINED_EXCLUDES works.

The ignored paths are fed directly into the underlying macOS watcher, so the fix in 6e8e175 is indeed correct.

https://github.com/parcel-bundler/watcher/blob/99cf43b739b25bbd3538506dbc4e87a202f7af84/src/macos/FSEventsBackend.cc#L223-L234

Note my previous comment, however. I think the ignore paths would be unnecessary if the watcher on ~/Library was being installed as non-recursive, which appears to be the original intention.

@deepak1556
Copy link
Collaborator

I think the ignore paths would be unnecessary if the watcher on ~/Library was being installed as non-recursive, which appears to be the original intention.

Makes sense, the api contract does define values with path separators are treated as recursive

vscode/src/vscode-dts/vscode.d.ts

Lines 13509 to 13512 in 673112c

* Additional paths can be added for file watching by providing a {@link RelativePattern} with
* a `base` path to watch. If the path is a folder and the `pattern` is complex (e.g. contains
* `**` or path segments), it will be watched recursively and otherwise will be watched
* non-recursively (i.e. only changes to the first level of the path will be reported).
. So maybe an explicit recursive option would be necessary to have a fix in
const parentWatcher = vscode.workspace.createFileSystemWatcher(glob, !listeners.create, true, !listeners.delete);

Thanks again for investigating and narrowing down the root cause! I will let @bpasero follow-up.

@deepak1556 deepak1556 reopened this Sep 3, 2024
@deepak1556 deepak1556 removed their assignment Sep 3, 2024
@vs-code-engineering vs-code-engineering bot removed the unreleased Patch has not yet been released in VS Code Insiders label Sep 3, 2024
@tmm1
Copy link
Contributor

tmm1 commented Sep 3, 2024

I misunderstood how vscode.RelativePattern works.. only the pattern is being checked, so ** or / makes sense.

So that's not the cause, but there is some other reason the ~/Library watch is being installed as recursive.

The pattern used for it is:

this.logger.trace(`Creating parent dir watcher for ${dirUri.toString()}`);
const glob = new vscode.RelativePattern(Utils.dirname(dirUri), Utils.basename(dirUri));

@tmm1
Copy link
Contributor

tmm1 commented Sep 3, 2024

Sorry for all the wild goose chasing here, but I confirmed this is a tsserver bug: microsoft/TypeScript#59831 (comment)

You can close this issue.

@bpasero
Copy link
Member

bpasero commented Sep 3, 2024

@tmm1 I noticed your PR in parcel-bundler/watcher#186. If you look at this code a few lines above your code:

https://github.com/parcel-bundler/watcher/blob/99cf43b739b25bbd3538506dbc4e87a202f7af84/src/macos/FSEventsBackend.cc#L96-L99

My change would cause any file path to be ignored that is in ~/Library/Containers, so the code you changed would not get hit for these paths.

Are you saying your fix is not helpful for parcel watcher?

@tmm1
Copy link
Contributor

tmm1 commented Sep 3, 2024

My change would cause any file path to be ignored that is in ~/Library/Containers, so the code you changed would not get hit for these paths.

Are you saying your fix is not helpful for parcel watcher?

You're absolutely right. I was mistaken. Thanks again for the quick fix!

@bpasero
Copy link
Member

bpasero commented Sep 3, 2024

@tmm1 recursive watchers are only set when the glob pattern portion contains a /, not if the path contains a /. We do this because if your glob pattern contains path segments, it means we have to watch recursively from the provided path.

Thanks again for all your analysis, closing then.

@bpasero bpasero closed this as completed Sep 3, 2024
@bpasero bpasero added the author-verification-requested Issues potentially verifiable by issue author label Sep 11, 2024
@bpasero bpasero added the verified Verification succeeded label Sep 27, 2024
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Oct 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
author-verification-requested Issues potentially verifiable by issue author bug Issue identified by VS Code Team member as probable bug file-watcher File watcher macos Issues with VS Code on MAC/OS X verified Verification succeeded
Projects
None yet
Development

No branches or pull requests