Skip to content

Potential fix for code scanning alert no. 28: Unsafe shell command constructed from library input #75

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.

Sign up for GitHub

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

Open
anupsv wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Potential fix for code scanning alert no. 28: Unsafe shell command constructed from library input #75

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
53aa006
Potential fix for code scanning alert no. 28: Unsafe shell command co…
anupsv Jan 23, 2026
06f71fc
Potential fix for code scanning alert no. 37: Second order command in…
anupsv Jan 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/sdk/src/client/common/templates/git.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

try {
// Clone with no checkout
await execAsync(`git clone --no-checkout --progress ${repoURL} ${targetDir}`, {
await execFileAsync("git", ["clone", "--no-checkout", "--progress", repoURL, targetDir], {

Check failure

Code scanning / CodeQL

Second order command injection High

Command line argument that depends on
library input
Loading
can execute an arbitrary command if --upload-pack is used with git.
Command line argument that depends on
library input
Loading
can execute an arbitrary command if --upload-pack is used with git.

Copilot Autofix

AI 29 days ago

In general, to fix second‑order git command injection you must ensure any user‑controlled values that become git repository URLs or clone destinations cannot be interpreted as git options. At a minimum, reject inputs that start with -, and ideally constrain them to known‑good schemes (e.g. https://, git@, ssh://, file://, or local paths). This validation must happen before passing values into execFile("git", [...]) (or equivalent), and must be applied consistently to all code paths that can receive tainted inputs.

For this codebase, the best targeted fix is:

  1. Strengthen validateRepoURL in packages/sdk/src/client/common/templates/git.ts so it:

    • Rejects strings starting with -.
    • Optionally enforces that if the value looks like a URL, it uses an allowed scheme.
    • Leaves existing behavior (accepting normal git/ssh/https URLs and local paths) intact.

  2. Ensure this validation also covers other git operations performed in this file (e.g. subdirectory/template fetch helpers, which are in the omitted [...]), by reusing validateRepoURL / validateTargetDir anywhere a potentially tainted repo URL or directory is passed to execFileAsync("git", ...). Since you’ve provided only the fetchTemplate snippet, we will confine edits to strengthening the validators there, which addresses the flagged sink at line 76 and any other code in this file that already calls these validators.

Concretely, in packages/sdk/src/client/common/templates/git.ts we will modify validateRepoURL to add:

  • A simple URL parse using the standard URL constructor when the string contains ://, and a check to allow only http:, https:, ssh:, or git: schemes.
  • A comment describing the security rationale, to clarify intent for future maintainers.

No other files (create.ts, index.ts) require code changes to address this specific injection risk; they will continue to pass cfg.repoURL down as before, but that value will now be validated in fetchTemplate.

Suggested changeset 1
packages/sdk/src/client/common/templates/git.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/sdk/src/client/common/templates/git.ts b/packages/sdk/src/client/common/templates/git.ts
--- a/packages/sdk/src/client/common/templates/git.ts
+++ b/packages/sdk/src/client/common/templates/git.ts
@@ -23,17 +23,32 @@
  * interpreted as an option such as `--upload-pack`.
  *
  * This allows typical git/ssh/https URLs and local paths, but rejects
- * values starting with a dash.
+ * values starting with a dash and disallows unexpected URL schemes.
  */
 function validateRepoURL(repoURL: string): void {
   if (!repoURL || typeof repoURL !== "string") {
     throw new Error("Invalid repository URL");
   }
 
-  // Disallow anything that looks like a git option
+  // Disallow anything that looks like a git option (e.g. "--upload-pack")
   if (repoURL.startsWith("-")) {
     throw new Error("Repository URL must not start with '-'");
   }
+
+  // If the value looks like a URL, restrict it to common git URL schemes.
+  // Local paths (no "://") are allowed as-is.
+  if (repoURL.includes("://")) {
+    try {
+      const parsed = new URL(repoURL);
+      const allowedProtocols = new Set(["http:", "https:", "ssh:", "git:"]);
+      if (!allowedProtocols.has(parsed.protocol)) {
+        throw new Error("Unsupported repository URL protocol");
+      }
+    } catch (err) {
+      // Re-throw with a generic validation message to avoid leaking details
+      throw new Error("Invalid repository URL");
+    }
+  }
 }
 
 /**
EOF
@@ -23,17 +23,32 @@
* interpreted as an option such as `--upload-pack`.
*
* This allows typical git/ssh/https URLs and local paths, but rejects
* values starting with a dash.
* values starting with a dash and disallows unexpected URL schemes.
*/
function validateRepoURL(repoURL: string): void {
if (!repoURL || typeof repoURL !== "string") {
throw new Error("Invalid repository URL");
}

// Disallow anything that looks like a git option
// Disallow anything that looks like a git option (e.g. "--upload-pack")
if (repoURL.startsWith("-")) {
throw new Error("Repository URL must not start with '-'");
}

// If the value looks like a URL, restrict it to common git URL schemes.
// Local paths (no "://") are allowed as-is.
if (repoURL.includes("://")) {
try {
const parsed = new URL(repoURL);
const allowedProtocols = new Set(["http:", "https:", "ssh:", "git:"]);
if (!allowedProtocols.has(parsed.protocol)) {
throw new Error("Unsupported repository URL protocol");
}
} catch (err) {
// Re-throw with a generic validation message to avoid leaking details
throw new Error("Invalid repository URL");
}
}
}

/**
Copilot is powered by AI and may make mistakes. Always verify output.

Check failure

Code scanning / CodeQL

Second order command injection High

Command line argument that depends on
library input
Loading
can execute an arbitrary command if --upload-pack is used with git.
Command line argument that depends on
library input
Loading
can execute an arbitrary command if --upload-pack is used with git.

Copilot Autofix

AI 29 days ago

In general, to fix this kind of issue you must ensure that any value that can affect git’s interpretation of command-line arguments (repository URL, extra options) is validated or constrained before being passed to execFile. For git, this means ensuring repository URLs are well-formed (e.g., starting with git@, ssh://, https://, or being a safe local path), and that no argument intended to be a path can be interpreted as a git option like --upload-pack.

In this codebase the best targeted fix is:

  1. Add a small validation helper in packages/sdk/src/client/common/templates/git.ts that:
    • Ensures repoURL is either a safe “URL-like” string (e.g., starts with git@, ssh://, https://, or file:) or a relative/local path without leading dashes.
    • Ensures targetDir is a safe filesystem path (non-empty, not starting with -, no embedded NUL, etc.).
  2. Call this helper at the beginning of fetchTemplate so that any attempt to pass in a malicious repoURL or targetDir (from options.template, template catalog, or project name) will be rejected before execFileAsync runs.

This approach:

  • Keeps functionality the same for legitimate inputs.
  • Centralizes validation at the point of interaction with git, so all callers (current and future) are covered.
  • Addresses both CodeQL alert variants (tainted targetDir and underlying tainted repoURL) with a single change.
  • Uses only standard Node APIs; no new external dependencies are needed.

No changes are required in create.ts or index.ts for the fix to be effective; those files will continue to pass through options as before, but now invalid values will be rejected by fetchTemplate.

Suggested changeset 1
packages/sdk/src/client/common/templates/git.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/sdk/src/client/common/templates/git.ts b/packages/sdk/src/client/common/templates/git.ts
--- a/packages/sdk/src/client/common/templates/git.ts
+++ b/packages/sdk/src/client/common/templates/git.ts
@@ -19,6 +19,39 @@
 }
 
 /**
+ * Basic validation to ensure the git repository URL or path cannot be
+ * interpreted as an option such as `--upload-pack`.
+ *
+ * This allows typical git/ssh/https URLs and local paths, but rejects
+ * values starting with a dash.
+ */
+function validateRepoURL(repoURL: string): void {
+  if (!repoURL || typeof repoURL !== "string") {
+    throw new Error("Invalid repository URL");
+  }
+
+  // Disallow anything that looks like a git option
+  if (repoURL.startsWith("-")) {
+    throw new Error("Repository URL must not start with '-'");
+  }
+}
+
+/**
+ * Validate the target directory used for cloning to ensure it cannot be
+ * interpreted by git as an option.
+ */
+function validateTargetDir(targetDir: string): void {
+  if (!targetDir || typeof targetDir !== "string") {
+    throw new Error("Invalid target directory");
+  }
+
+  // Disallow leading dashes so it cannot be parsed as an option
+  if (targetDir.startsWith("-")) {
+    throw new Error("Target directory must not start with '-'");
+  }
+}
+
+/**
  * Fetch full template repository
  */
 export async function fetchTemplate(
@@ -32,6 +65,10 @@
     throw new Error("repoURL is required");
   }
 
+  // Validate untrusted inputs before passing to git
+  validateRepoURL(repoURL);
+  validateTargetDir(targetDir);
+
   logger.info(`\nCloning repo: ${repoURL} → ${targetDir}\n`);
 
   try {
EOF
@@ -19,6 +19,39 @@
}

/**
* Basic validation to ensure the git repository URL or path cannot be
* interpreted as an option such as `--upload-pack`.
*
* This allows typical git/ssh/https URLs and local paths, but rejects
* values starting with a dash.
*/
function validateRepoURL(repoURL: string): void {
if (!repoURL || typeof repoURL !== "string") {
throw new Error("Invalid repository URL");
}

// Disallow anything that looks like a git option
if (repoURL.startsWith("-")) {
throw new Error("Repository URL must not start with '-'");
}
}

/**
* Validate the target directory used for cloning to ensure it cannot be
* interpreted by git as an option.
*/
function validateTargetDir(targetDir: string): void {
if (!targetDir || typeof targetDir !== "string") {
throw new Error("Invalid target directory");
}

// Disallow leading dashes so it cannot be parsed as an option
if (targetDir.startsWith("-")) {
throw new Error("Target directory must not start with '-'");
}
}

/**
* Fetch full template repository
*/
export async function fetchTemplate(
@@ -32,6 +65,10 @@
throw new Error("repoURL is required");
}

// Validate untrusted inputs before passing to git
validateRepoURL(repoURL);
validateTargetDir(targetDir);

logger.info(`\nCloning repo: ${repoURL} → ${targetDir}\n`);

try {
Copilot is powered by AI and may make mistakes. Always verify output.
@anupsv anupsv committed this autofix suggestion 29 days ago.
maxBuffer: 10 * 1024 * 1024,
});

Expand Down