Merge pull request #70 from abhinavgautam01/fix/issue-66-git-argument-injection

Author: antfu

plugins/git/src/node/git.ts

@@ -41,6 +41,14 @@ export async function tryGit(cwd: string, args: string[]): Promise<string | null
   }
 }
 
+/**
+ * Client-supplied revisions must not start with `-`, which Git would parse as
+ * an option before it treats the value as a revision.
+ */
+export function isSafeRevision(rev: string): boolean {
+  return rev.length > 0 && !rev.startsWith('-')
+}
+
 /** Resolve the repository root for `cwd`, or `null` when `cwd` is outside a repo. */
 export async function resolveRepoRoot(cwd: string): Promise<string | null> {
   return tryGit(cwd, ['rev-parse', '--show-toplevel'])

plugins/git/src/rpc/functions/log.ts

@@ -1,5 +1,5 @@
 import { defineRpcFunction } from 'devframe'
-import { RECORD, splitClean, tryGit, UNIT } from '../../node/git.ts'
+import { isSafeRevision, RECORD, splitClean, tryGit, UNIT } from '../../node/git.ts'
 import { getGitContext } from '../context.ts'
 
 export interface Commit {
@@ -108,8 +108,11 @@ export const log = defineRpcFunction({
           `--skip=${skip}`,
           `--pretty=format:${FORMAT}`,
         ]
-        if (ref)
-          command.push(ref)
+        if (ref) {
+          if (!isSafeRevision(ref))
+            return { isRepo: true, commits: [], limit, skip, hasMore: false }
+          command.push('--end-of-options', ref)
+        }
 
         const raw = await tryGit(git.cwd, command)
         // `null` happens on a repo with no commits yet — treat as empty.

plugins/git/src/rpc/functions/show.ts

@@ -1,6 +1,6 @@
 import type { GitContext } from '../context.ts'
 import { defineRpcFunction } from 'devframe'
-import { splitClean, tryGit, UNIT } from '../../node/git.ts'
+import { isSafeRevision, splitClean, tryGit, UNIT } from '../../node/git.ts'
 import { getGitContext } from '../context.ts'
 
 /** Hard cap on the returned patch text to keep payloads bounded. */
@@ -102,7 +102,10 @@ function parseNumstat(raw: string): CommitFile[] {
 }
 
 async function readCommit(git: GitContext, hash: string, includePatch: boolean): Promise<CommitDetail> {
-  const meta = await tryGit(git.cwd, ['show', '-s', `--format=${SHOW_FORMAT}`, hash])
+  if (!isSafeRevision(hash))
+    return { ...EMPTY_DETAIL, isRepo: true }
+
+  const meta = await tryGit(git.cwd, ['show', '-s', `--format=${SHOW_FORMAT}`, '--end-of-options', hash])
   if (meta == null)
     return { ...EMPTY_DETAIL, isRepo: true }
 
@@ -122,15 +125,15 @@ async function readCommit(git: GitContext, hash: string, includePatch: boolean):
   ] = meta.split(UNIT)
 
   // `--root` so the initial commit reports its full tree as additions.
-  const numstat = await tryGit(git.cwd, ['diff-tree', '--no-commit-id', '--numstat', '-r', '--root', hash])
+  const numstat = await tryGit(git.cwd, ['diff-tree', '--no-commit-id', '--numstat', '-r', '--root', '--end-of-options', hash])
   const files = numstat ? parseNumstat(numstat) : []
   const totalAdditions = files.reduce((sum, f) => sum + f.additions, 0)
   const totalDeletions = files.reduce((sum, f) => sum + f.deletions, 0)
 
   let patch: string | null = null
   let truncated = false
   if (includePatch) {
-    const raw = await tryGit(git.cwd, ['diff-tree', '-p', '--no-commit-id', '-r', '--root', hash])
+    const raw = await tryGit(git.cwd, ['diff-tree', '-p', '--no-commit-id', '-r', '--root', '--end-of-options', hash])
     if (raw != null) {
       if (raw.length > PATCH_CHAR_LIMIT) {
         patch = raw.slice(0, PATCH_CHAR_LIMIT)

plugins/git/test/git.test.ts

@@ -1,4 +1,6 @@
-import type { CommitResult, GitBranches, GitDiff, GitLog, GitStatus } from '../src/index'
+import type { CommitDetail, CommitResult, GitBranches, GitDiff, GitLog, GitStatus } from '../src/index'
+import { existsSync } from 'node:fs'
+import { join } from 'node:path'
 import { createRpcClient } from 'devframe/rpc/client'
 import { collectStaticRpcDump } from 'devframe/rpc/dump'
 import { createWsRpcChannel } from 'devframe/rpc/transports/ws-client'
@@ -83,6 +85,41 @@ describe('@devframes/plugin-git', () => {
     expect(tail.hasMore).toBe(false)
   })
 
+  it('treats dashed log refs as invalid revisions instead of Git options', async () => {
+    const rpc = bootRpc(server.port)
+    const marker = join(repo.dir, 'log-injected.txt')
+
+    const log = await rpc.$call('git:log', { ref: `--output=${marker}` }) as GitLog
+
+    expect(log.isRepo).toBe(true)
+    expect(log.commits).toEqual([])
+    expect(log.hasMore).toBe(false)
+    expect(existsSync(marker)).toBe(false)
+  })
+
+  it('treats dashed show hashes as invalid revisions instead of Git options', async () => {
+    const rpc = bootRpc(server.port)
+    const marker = join(repo.dir, 'show-injected.txt')
+
+    const detail = await rpc.$call('git:show', { hash: `--output=${marker}` }) as CommitDetail
+
+    expect(detail.isRepo).toBe(true)
+    expect(detail.found).toBe(false)
+    expect(existsSync(marker)).toBe(false)
+  })
+
+  it('returns commit details for a valid hash', async () => {
+    const rpc = bootRpc(server.port)
+    const log = await rpc.$call('git:log', { limit: 1 }) as GitLog
+
+    const detail = await rpc.$call('git:show', { hash: log.commits[0].hash }) as CommitDetail
+
+    expect(detail.isRepo).toBe(true)
+    expect(detail.found).toBe(true)
+    expect(detail.hash).toBe(log.commits[0].hash)
+    expect(detail.files.map(file => file.path)).toContain('a.txt')
+  })
+
   it('lists local branches with the current one first', async () => {
     const rpc = bootRpc(server.port)
     const result = await rpc.$call('git:branches', {}) as GitBranches