Skip to content

Support trust gvfs idx files for scalar prefetch#766

Closed
tyrielv wants to merge 1 commit intomicrosoft:vfs-2.49.0from
tyrielv:gvfs-helper-trust-pack-index
Closed

Support trust gvfs idx files for scalar prefetch#766
tyrielv wants to merge 1 commit intomicrosoft:vfs-2.49.0from
tyrielv:gvfs-helper-trust-pack-index

Conversation

@tyrielv
Copy link
Copy Markdown

@tyrielv tyrielv commented Jun 11, 2025

GVFS protocol's prefetch operation (used to fetch pack files for commits and trees) includes idx files as well as pack files. Gvfs.exe (from VFSForGit) trusts the idx files it fetches, but scalar currently does not and always reindexes them locally.
This has an observed performance impact where scalar clone takes up to 10x as long as gvfs clone for large repositories.

This pull request adds support for trusting idx files to the prefetch operation in scalar, with the following behavior in priority order:

  1. If --[no-]trust-idx is specified, use that
  2. If global config gvfs.trustidxfiles is set, use that
  3. If the url to clone/prefetch is Azure DevOps, trust it
  4. Otherwise, do not trust it

@tyrielv tyrielv force-pushed the gvfs-helper-trust-pack-index branch 5 times, most recently from c41a01f to e8b8eaf Compare June 11, 2025 20:26
GVFS protocol's prefetch operation (used to fetch pack files for commits
and trees) includes idx files as well as pack files. Gvfs.exe (from
VFSForGit) trusts the idx files it fetches, but scalar currently does
not and always reindexes them locally.
This has an observed performance impact where scalar clone takes up to
10x as long as gvfs clone for large repositories.

This pull request adds support for trusting idx files to the prefetch
operation in scalar, with the following behavior in priority order:
1. If --[no-]trust-idx is specified, use that
2. If global config `gvfs.trustidxfiles` is set, use that
3. If the url to clone/prefetch is Azure DevOps, trust it
4. Otherwise, do not trust it
@tyrielv tyrielv force-pushed the gvfs-helper-trust-pack-index branch from e8b8eaf to 5cba6be Compare June 11, 2025 20:32
@tyrielv tyrielv marked this pull request as ready for review June 12, 2025 17:34
Copy link
Copy Markdown

@derrickstolee derrickstolee left a comment

Choose a reason for hiding this comment

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

I'm sorry that you spent time on this, but trusting the .idx files is a serious security issue. If we "trust" the .idx file, then we could have a rogue cache server deliver us a bad .idx file where it says "this Object Id has this content" but that content doesn't actually hash to that object ID.

There are similar checks when asking for a loose object by object ID: we used to trust the cache server for performance reasons, but this security boundary is really really important.

@tyrielv tyrielv closed this Jun 12, 2025
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.

2 participants