Skip to content

Add expansion of short paths using native Windows call #4068

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

Merged
merged 1 commit into from
Jul 4, 2025

Conversation

koesie10
Copy link
Member

@koesie10 koesie10 commented Jul 2, 2025

It seems like in certain cases we cannot resolve Windows short paths and fail with an error message in the CodeQL CLI. As a fallback mechanism, this adds a native call to the GetLongPathNameW win32 API function, which will correctly resolve the path if it exists. To reduce the cost of calling this function, we are now also only expanding the short paths once instead of twice per variant analysis run.

To call this function, we're using koffi. koffi includes a native Node.js extension for calling into the C functions and bundles this extension for a lot of platforms by default:

Screenshot 2025-07-02 at 14 30 16

This would increase the size of our extension by tens of megabytes (all native extensions together are about 66 MB), so I've worked around this by only bundling Windows x64 by default. The call is only made on Windows, and I expect x64 to be the only common Windows platform. In total, this adds about 2.2MB to the extension.

It's been tested by the user that reported the issue and this seems to have fixed the issue (#4059 (comment)), and I've also tested it on one of my own Windows machines where it resolves the path correctly (although the original workaround also works there, so I've artificially made it use the native method).

@koesie10 koesie10 marked this pull request as ready for review July 2, 2025 12:55
@Copilot Copilot AI review requested due to automatic review settings July 2, 2025 12:55
@koesie10 koesie10 requested review from a team as code owners July 2, 2025 12:55
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@LWSimpkins
Copy link

I was investigating this issue for the forked version we use and was going to submit a PR, but this was already in progress!

For some background about what caused this:

The bug does not seem to affect directories with spaces in the name, only ones without spaces. That's why the unit test for C:\\PROGRA~1 / C:\\Program Files did not catch this in my local testing. Example from local testing with C:\\Users\\LSIMPK~1 with 22.15.1 on my PATH:

stats:

Directory stats.dev stats.ino
C:\Users\LSIMPK~1 0n 6473924466378324n

childStats:

Directory stats.dev stats.ino
C:\Users\All Users 3705393598n 3096224745883570n
C:\Users\Default 0n 2251799815715884n
C:\Users\Default User 3705393598n 3659174699303827n
C:\Users\desktop.ini 0n 2251799815738381n
C:\Users\lsimpkins 0n 6473924466378324n

Copy link
Contributor

@charisk charisk left a comment

Choose a reason for hiding this comment

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

LGTM

@koesie10
Copy link
Member Author

koesie10 commented Jul 4, 2025

@LWSimpkins Thanks for the context! It's interesting that I can't reproduce this on my Windows machine. I've created a test account vscode-testing-accou (which seems to be the maximum username length), and the logs show that it can find the inode:

Expanding short paths in: C:\Users\VSCODE~1\AppData\Local\Temp\queries_-4136-bvplmSDGNe9A
dir: C:\
base: Users
Component is not a short name
dir: C:\Users
base: VSCODE~1
Expanding short path component: VSCODE~1
dev/inode: 210460886/1407374883597676
considering child: All Users
child dev/inode: 210460886/281474976750657
considering child: Default
child dev/inode: 210460886/281474976712737
considering child: Default User
child dev/inode: 210460886/562949953460240
considering child: desktop.ini
child dev/inode: 210460886/281474976749587
considering child: koen
Error reading stats for child: Error: EPERM: operation not permitted, lstat 'C:\Users\koen'
considering child: Public
child dev/inode: 210460886/281474976712785
considering child: vscode-testing-accou
child dev/inode: 210460886/1407374883597676
Found a match: vscode-testing-accou
dir: C:\Users\vscode-testing-accou
base: AppData
Component is not a short name
dir: C:\Users\vscode-testing-accou\AppData
base: Local
Component is not a short name
dir: C:\Users\vscode-testing-accou\AppData\Local
base: Temp
Component is not a short name
dir: C:\Users\vscode-testing-accou\AppData\Local\Temp
base: queries_-4136-bvplmSDGNe9A
Component is not a short name

@koesie10 koesie10 merged commit 383405c into main Jul 4, 2025
72 of 73 checks passed
@koesie10 koesie10 deleted the koesie10/expand-short-paths branch July 4, 2025 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CodeQL Extension submits empty block to multi-repository variant analysis (MRVA)
3 participants