Skip to content

Commit b1913f4

Browse files
fix(#3577): remove shell=True and convert commands to argument lists to prevent command injection
Signed-off-by: guptapratykshh <pratykshgupta9999@gmail.com>
1 parent 7122c73 commit b1913f4

File tree

3 files changed

+26
-33
lines changed

3 files changed

+26
-33
lines changed

augur/tasks/git/util/facade_worker/facade_worker/config.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -257,15 +257,15 @@ def insert_or_update_data(self, query, **bind_args)-> None:
257257
def inc_repos_processed(self):
258258
self.repos_processed += 1
259259

260-
def run_git_command(self, cmd: str, timeout: int, capture_output: bool = False, operation_description: str = None) -> tuple:
260+
def run_git_command(self, cmd: list, timeout: int, capture_output: bool = False, operation_description: str = None) -> tuple:
261261
"""
262262
Execute a git command with timeout handling.
263263
264264
This method provides a unified interface for running git commands with
265265
consistent timeout handling and error logging across all facade operations.
266266
267267
Args:
268-
cmd: The git command to execute
268+
cmd: The git command to execute as a list of arguments (e.g., ["git", "clone", url])
269269
timeout: Timeout in seconds
270270
capture_output: If True, capture stdout/stderr; if False, discard them
271271
operation_description: Human-readable description for error logging
@@ -277,12 +277,11 @@ def run_git_command(self, cmd: str, timeout: int, capture_output: bool = False,
277277
stdout_content is empty string if capture_output=False
278278
"""
279279
if operation_description is None:
280-
operation_description = cmd
280+
operation_description = ' '.join(cmd)
281281

282282
try:
283283
# Common options for all subprocess.run calls
284284
run_options = {
285-
'shell': True,
286285
'timeout': timeout,
287286
'check': False
288287
}

augur/tasks/git/util/facade_worker/facade_worker/repofetch.py

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ def git_repo_initialize(facade_helper, session, repo_git):
148148

149149
facade_helper.log_activity('Verbose', f"Cloning: {git}")
150150

151-
cmd = f"git -C {repo_path} clone '{git}' {repo_name}"
151+
cmd = ["git", "-C", repo_path, "clone", git, repo_name]
152152
return_code, _ = facade_helper.run_git_command(
153153
cmd,
154154
timeout=7200, # 2 hours for large repos
@@ -320,7 +320,7 @@ def git_repo_updates(facade_helper, repo_git):
320320

321321
try:
322322

323-
firstpull = (f"git -C {absolute_path} pull")
323+
firstpull = ["git", "-C", absolute_path, "pull"]
324324

325325
return_code_remote, _ = facade_helper.run_git_command(
326326
firstpull,
@@ -340,35 +340,32 @@ def git_repo_updates(facade_helper, repo_git):
340340

341341
# session.log_activity('Verbose', f'remote default is {logremotedefault}.')
342342

343-
getremotedefault = (
344-
f"git -C {absolute_path} remote show origin | sed -n '/HEAD branch/s/.*: //p'")
343+
getremotedefault = ["git", "-C", absolute_path, "symbolic-ref", "refs/remotes/origin/HEAD"]
345344

346345
return_code_remote, remotedefault = facade_helper.run_git_command(
347346
getremotedefault,
348-
timeout=60, # 1 minute for remote query
347+
timeout=60,
349348
capture_output=True,
350349
operation_description='get remote default branch'
351350
)
351+
if return_code_remote == 0 and remotedefault:
352+
remotedefault = remotedefault.split('/')[-1]
352353

353354
facade_helper.log_activity(
354355
'Verbose', f'remote default getting checked out is: {remotedefault}.')
355356

356-
getremotedefault = (
357-
f"git -C {absolute_path} checkout {remotedefault}")
358-
359-
facade_helper.log_activity(
360-
'Verbose', f"get remote default command is: \n \n {getremotedefault} \n \n ")
357+
checkout_cmd = ["git", "-C", absolute_path, "checkout", remotedefault]
361358

362359
return_code_remote_default_again, _ = facade_helper.run_git_command(
363-
getremotedefault,
360+
checkout_cmd,
364361
timeout=600, # 10 minutes for git checkout
365362
capture_output=False,
366363
operation_description=f'git checkout {remotedefault}'
367364
)
368365

369366
if return_code_remote_default_again == 0:
370367
facade_helper.log_activity('Verbose', "local checkout worked.")
371-
cmd = (f"git -C {absolute_path} pull")
368+
cmd = ["git", "-C", absolute_path, "pull"]
372369

373370
return_code, _ = facade_helper.run_git_command(
374371
cmd,
@@ -384,7 +381,7 @@ def git_repo_updates(facade_helper, repo_git):
384381

385382
finally:
386383

387-
cmd = (f"git -C {absolute_path} pull")
384+
cmd = ["git", "-C", absolute_path, "pull"]
388385

389386
return_code, _ = facade_helper.run_git_command(
390387
cmd,
@@ -411,23 +408,23 @@ def git_repo_updates(facade_helper, repo_git):
411408

412409
# session.log_activity('Verbose', f'remote default is {logremotedefault}.')
413410

414-
getremotedefault = (
415-
f"git -C {absolute_path} remote show origin | sed -n '/HEAD branch/s/.*: //p'")
411+
getremotedefault = ["git", "-C", absolute_path, "symbolic-ref", "refs/remotes/origin/HEAD"]
416412

417413
return_code_remote, remotedefault = facade_helper.run_git_command(
418414
getremotedefault,
419-
timeout=60, # 1 minute for remote query
415+
timeout=60,
420416
capture_output=True,
421417
operation_description='get remote default branch'
422418
)
419+
if return_code_remote == 0 and remotedefault:
420+
remotedefault = remotedefault.split('/')[-1]
423421

424422
try:
425423

426-
getremotedefault = (
427-
f"git -C {absolute_path} checkout {remotedefault}")
424+
checkout_cmd = ["git", "-C", absolute_path, "checkout", remotedefault]
428425

429426
return_code_remote_default, _ = facade_helper.run_git_command(
430-
getremotedefault,
427+
checkout_cmd,
431428
timeout=600, # 10 minutes for git checkout
432429
capture_output=False,
433430
operation_description=f'git checkout {remotedefault}'
@@ -436,7 +433,7 @@ def git_repo_updates(facade_helper, repo_git):
436433
facade_helper.log_activity(
437434
'Verbose', f'get remote default result (return code): {return_code_remote_default}')
438435

439-
getcurrentbranch = (f"git -C {absolute_path} branch")
436+
getcurrentbranch = ["git", "-C", absolute_path, "branch"]
440437

441438
return_code_local, localdefault = facade_helper.run_git_command(
442439
getcurrentbranch,
@@ -448,8 +445,7 @@ def git_repo_updates(facade_helper, repo_git):
448445
facade_helper.log_activity(
449446
'Verbose', f'remote default is: {remotedefault}, and localdefault is {localdefault}.')
450447

451-
cmd_checkout_default = (
452-
f"git -C {absolute_path} checkout {remotedefault}")
448+
cmd_checkout_default = ["git", "-C", absolute_path, "checkout", remotedefault]
453449

454450
cmd_checkout_default_wait, _ = facade_helper.run_git_command(
455451
cmd_checkout_default,
@@ -458,9 +454,9 @@ def git_repo_updates(facade_helper, repo_git):
458454
operation_description=f'git checkout {remotedefault}'
459455
)
460456

461-
cmdpull2 = (f"git -C {absolute_path} pull")
457+
cmdpull2 = ["git", "-C", absolute_path, "pull"]
462458

463-
cmd_reset = (f"git -C {absolute_path} reset --hard origin/{remotedefault}")
459+
cmd_reset = ["git", "-C", absolute_path, "reset", "--hard", f"origin/{remotedefault}"]
464460

465461
cmd_reset_wait, _ = facade_helper.run_git_command(
466462
cmd_reset,
@@ -469,7 +465,7 @@ def git_repo_updates(facade_helper, repo_git):
469465
operation_description=f'git reset --hard origin/{remotedefault}'
470466
)
471467

472-
cmd_clean = (f"git -C {absolute_path} clean -df")
468+
cmd_clean = ["git", "-C", absolute_path, "clean", "-df"]
473469

474470
return_code_clean, _ = facade_helper.run_git_command(
475471
cmd_clean,
@@ -483,9 +479,7 @@ def git_repo_updates(facade_helper, repo_git):
483479
facade_helper.log_activity('Verbose', f'Second pass failed: {e}.')
484480
pass
485481

486-
cmdpull2 = (f"git -C {absolute_path} pull")
487-
488-
print(cmdpull2)
482+
cmdpull2 = ["git", "-C", absolute_path, "pull"]
489483
return_code, _ = facade_helper.run_git_command(
490484
cmdpull2,
491485
timeout=600, # 10 minutes for git pull

augur/tasks/git/util/facade_worker/facade_worker/utilitymethods.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ def get_absolute_repo_path(repo_base_dir, repo_id, repo_path,repo_name):
107107

108108
def get_parent_commits_set(absolute_repo_path, facade_helper, logger=None):
109109

110-
cmd = "git --git-dir %s log --ignore-missing --pretty=format:'%%H'" % (absolute_repo_path)
110+
cmd = ["git", "--git-dir", absolute_repo_path, "log", "--ignore-missing", "--pretty=format:%H"]
111111

112112
# Use facade_helper's unified git command runner
113113
return_code, stdout = facade_helper.run_git_command(

0 commit comments

Comments
 (0)