fix(git-fetcher): reject non-SHA commit values before invoking git (#11967)

`fetching/git-fetcher/src/index.ts` passed the lockfile-controlled `resolution.commit` value straight to `git fetch --depth 1 origin <commit>` and `git checkout <commit>` with no `--` separator and no format validation. A malicious `pnpm-lock.yaml` could put a value such as `--upload-pack=touch /tmp/pwned` in `resolution.commit`; `git` parses anything starting with `-` as an option, and on SSH or local-file transports `--upload-pack` runs the supplied command as the user running `pnpm install`. HTTPS ignores `--upload-pack`, but the SSH/file paths are enough to reach code execution.

The fix validates `resolution.commit` against `/^[0-9a-f]{40}$/i` at the entry of the fetcher and throws `INVALID_GIT_COMMIT` otherwise. This is strictly stronger than adding a `--` separator — a validated value cannot start with `-` or contain shell-significant characters at all.

Pacquet's `pacquet-git-fetcher` crate shells out to `git` along the same code path (`pacquet/crates/git-fetcher/src/fetcher.rs`) and had the identical issue. Ported the same check there, with a new `GitFetcherError::InvalidCommit` variant carrying the `INVALID_GIT_COMMIT` diagnostic code.

Reported by [AutoFyn](https://github.com/SignalPilot-Labs/AutoFyn).

.changeset/git-fetcher-reject-non-sha-commits.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/fetching.git-fetcher": patch
+"pnpm": patch
+---
+
+Reject git resolutions whose `commit` field is not a 40-character hexadecimal SHA before invoking `git`. A malicious lockfile could otherwise smuggle a value such as `--upload-pack=<command>` through `git fetch` / `git checkout`, which on SSH or local-file transports executes the supplied command.

fetching/git-fetcher/src/index.ts

@@ -26,6 +26,9 @@ export function createGitFetcher (createOpts: CreateGitFetcherOptions): { git: G
   const ignoreScripts = createOpts.ignoreScripts ?? false
 
   const gitFetcher: GitFetcher = async (cafs, resolution, opts) => {
+    if (!isValidCommitHash(resolution.commit)) {
+      throw new PnpmError('INVALID_GIT_COMMIT', `Invalid git commit hash "${resolution.commit}" for repository "${resolution.repo}". Expected a 40-character hexadecimal SHA.`)
+    }
     const tempLocation = await cafs.tempDir()
     if (allowedHosts.size > 0 && shouldUseShallow(resolution.repo, allowedHosts)) {
       await execGit(['init'], { cwd: tempLocation })
@@ -78,6 +81,10 @@ export function createGitFetcher (createOpts: CreateGitFetcherOptions): { git: G
   }
 }
 
+function isValidCommitHash (commit: string): boolean {
+  return /^[0-9a-f]{40}$/i.test(commit)
+}
+
 function shouldUseShallow (repoUrl: string, allowedHosts: Set<string>): boolean {
   try {
     const { host } = new URL(repoUrl)

fetching/git-fetcher/test/index.ts

@@ -227,7 +227,7 @@ test('fail when preparing a git-hosted package', async () => {
   ).rejects.toThrow('Failed to prepare git-hosted package fetched from "https://github.com/pnpm-e2e/prepare-script-fails.git": @pnpm.e2e/prepare-script-fails@1.0.0 npm-install: `npm install`')
 })
 
-test('fail when preparing a git-hosted package with a partial commit', async () => {
+test('reject a partial commit before invoking git', async () => {
   const storeDir = temporaryDirectory()
   const fetch = createGitFetcher({
     storeIndex: createStoreIndex(storeDir),
@@ -241,7 +241,24 @@ test('fail when preparing a git-hosted package with a partial commit', async ()
       }, {
         filesIndexFile: path.join(storeDir, 'index.json'),
       })
-  ).rejects.toThrow(/received commit [0-9a-f]{40} does not match expected value deadbeef/)
+  ).rejects.toThrow('Invalid git commit hash "deadbeef"')
+  expect(jest.mocked(execa)).not.toHaveBeenCalled()
+})
+
+test('reject a commit value that looks like a git option', async () => {
+  const storeDir = temporaryDirectory()
+  const fetch = createGitFetcher({ storeIndex: createStoreIndex(storeDir) }).git
+  await expect(
+    fetch(createCafsStore(storeDir),
+      {
+        commit: '--upload-pack=touch /tmp/pwned',
+        repo: 'file:///tmp/repo.git',
+        type: 'git',
+      }, {
+        filesIndexFile: path.join(storeDir, 'index.json'),
+      })
+  ).rejects.toThrow('Invalid git commit hash "--upload-pack=touch /tmp/pwned"')
+  expect(jest.mocked(execa)).not.toHaveBeenCalled()
 })
 
 test('do not build the package when scripts are ignored', async () => {

pacquet/crates/git-fetcher/src/error.rs

@@ -89,6 +89,17 @@ pub enum GitFetcherError {
     #[diagnostic(code(GIT_CHECKOUT_FAILED))]
     CheckoutMismatch { expected: String, received: String },
 
+    /// `resolution.commit` is not a 40-character hexadecimal SHA. A
+    /// commit value beginning with `-` would otherwise be parsed by
+    /// `git fetch` / `git checkout` as an option (e.g. `--upload-pack`),
+    /// allowing a malicious lockfile to execute arbitrary commands on
+    /// SSH or local-file transports.
+    #[display(
+        "Invalid git commit hash {commit:?} for repository {repo:?}. Expected a 40-character hexadecimal SHA."
+    )]
+    #[diagnostic(code(INVALID_GIT_COMMIT))]
+    InvalidCommit { commit: String, repo: String },
+
     #[display("I/O error during git fetch: {_0}")]
     #[diagnostic(code(pacquet_git_fetcher::io))]
     Io(#[error(source)] std::io::Error),

pacquet/crates/git-fetcher/src/fetcher.rs

@@ -106,6 +106,12 @@ impl<'a> GitFetcher<'a> {
     }
 
     fn run_sync<Reporter: self::Reporter>(self) -> Result<GitFetchOutput, GitFetcherError> {
+        if !is_valid_commit_hash(self.commit) {
+            return Err(GitFetcherError::InvalidCommit {
+                commit: self.commit.to_string(),
+                repo: self.repo.to_string(),
+            });
+        }
         let temp = tempfile::tempdir().map_err(GitFetcherError::Io)?;
         let temp_location = temp.path();
 
@@ -226,6 +232,13 @@ fn wrap_prepare_error(_repo: &str, err: PreparePackageError) -> GitFetcherError
     GitFetcherError::Prepare(err)
 }
 
+/// True iff `commit` is exactly a 40-character hexadecimal git SHA.
+/// Rejects everything else (short SHAs, ref names, option-shaped
+/// strings like `--upload-pack=…`) before the value reaches `git`.
+fn is_valid_commit_hash(commit: &str) -> bool {
+    commit.len() == 40 && commit.bytes().all(|b| b.is_ascii_hexdigit())
+}
+
 /// True iff `repo` parses to a host that pacquet should clone via the
 /// shallow `init` + `fetch --depth 1` path. Mirrors upstream's
 /// [`shouldUseShallow`](https://github.com/pnpm/pnpm/blob/94240bc046/fetching/git-fetcher/src/index.ts#L81-L91).

pacquet/crates/git-fetcher/src/fetcher/tests.rs

@@ -1,4 +1,4 @@
-use super::{GitFetcher, exec_git, extract_host, should_use_shallow};
+use super::{GitFetcher, exec_git, extract_host, is_valid_commit_hash, should_use_shallow};
 use crate::error::GitFetcherError;
 use pacquet_executor::ScriptsPrependNodePath;
 use pacquet_reporter::SilentReporter;
@@ -84,6 +84,55 @@ fn should_use_shallow_matches_known_host() {
     assert!(!should_use_shallow("https://example.com/x/y.git", &hosts));
 }
 
+#[test]
+fn is_valid_commit_hash_accepts_full_sha() {
+    assert!(is_valid_commit_hash("c9b30e71d704cd30fa71f2edd1ecc7dcc4985493"));
+    assert!(is_valid_commit_hash("C9B30E71D704CD30FA71F2EDD1ECC7DCC4985493"));
+}
+
+#[test]
+fn is_valid_commit_hash_rejects_short_or_option_shaped_values() {
+    assert!(!is_valid_commit_hash("deadbeef"));
+    assert!(!is_valid_commit_hash(""));
+    assert!(!is_valid_commit_hash("--upload-pack=touch /tmp/pwned"));
+    assert!(!is_valid_commit_hash("c9b30e71d704cd30fa71f2edd1ecc7dcc4985493 "));
+    // 40 chars but contains a non-hex digit.
+    assert!(!is_valid_commit_hash("c9b30e71d704cd30fa71f2edd1ecc7dcc498549z"));
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn fetcher_rejects_option_shaped_commit() {
+    let store_root = tempdir().unwrap();
+    let store_dir = StoreDir::from(store_root.path().to_path_buf());
+    let err = GitFetcher {
+        repo: "file:///tmp/githost",
+        commit: "--upload-pack=touch /tmp/pwned",
+        path: None,
+        git_shallow_hosts: &[],
+        allow_build: deny_all_builds(),
+        ignore_scripts: false,
+        unsafe_perm: true,
+        user_agent: None,
+        scripts_prepend_node_path: ScriptsPrependNodePath::Never,
+        script_shell: None,
+        node_execpath: None,
+        npm_execpath: None,
+        store_dir: &store_dir,
+        package_id: "pkg@1.0.0",
+        requester: "/test",
+        store_index_writer: None,
+        files_index_file: "pkg@1.0.0\tbuilt",
+        git_bin: None,
+    }
+    .run::<SilentReporter>()
+    .await
+    .unwrap_err();
+    assert!(
+        matches!(err, GitFetcherError::InvalidCommit { .. }),
+        "expected InvalidCommit, got {err:?}",
+    );
+}
+
 #[test]
 fn extract_host_handles_user_authority_and_port() {
     assert_eq!(extract_host("https://github.com/foo/bar"), Some("github.com"));