Skip to content

Install GitHub plugins when Git is unavailable #127

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
balki97 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Install GitHub plugins when Git is unavailable #127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
75 changes: 70 additions & 5 deletions src/main/plugin-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,21 @@ import { ipcMain } from 'electron';
import { execFile } from 'child_process';
import * as path from 'path';
import * as fs from 'fs';
import * as os from 'os';
import { getPluginsDir, restartPython } from './python';
import { parseGitHubRepository, PLUGIN_SOURCE_FILE } from './plugin-source';

// Run git with an explicit argv array — never via a shell. This removes the
// OS command-injection vector that `exec(`git clone ${gitUrl} ...`)` had:
// gitUrl/name are no longer interpolated into a shell string.
function execFileAsync(file: string, args: string[], cwd?: string): Promise<string> {
return new Promise((resolve, reject) => {
execFile(file, args, { cwd, timeout: 60000 }, (error, stdout, stderr) => {
if (error) reject(new Error(stderr || error.message));
if (error) {
const failure = new Error(stderr.trim() || error.message) as NodeJS.ErrnoException;
failure.code = (error as NodeJS.ErrnoException).code;
reject(failure);
}
else resolve(stdout.trim());
});
});
Expand Down Expand Up @@ -55,10 +61,38 @@ interface InstalledPlugin {
name: string;
path: string;
hasGit: boolean;
canUpdate: boolean;
manifest: any | null;
version: string;
}

function readArchiveSource(pluginDir: string): string | null {
try {
const value = JSON.parse(fs.readFileSync(path.join(pluginDir, PLUGIN_SOURCE_FILE), 'utf8'))?.url;
return typeof value === 'string' && parseGitHubRepository(value) ? value : null;
} catch {
return null;
}
}

async function installGitHubArchive(gitUrl: string, targetDir: string): Promise<void> {
const repository = parseGitHubRepository(gitUrl);
if (!repository) throw new Error('archive fallback only supports public github.com repositories');

const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'feedback-plugin-'));
const archivePath = path.join(tempDir, 'plugin.tar.gz');
try {
const response = await fetch(`https://codeload.github.com/${repository.owner}/${repository.repo}/tar.gz/HEAD`);
if (!response.ok) throw new Error(`GitHub download returned HTTP ${response.status}`);
fs.writeFileSync(archivePath, Buffer.from(await response.arrayBuffer()));
Comment on lines +85 to +87

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant plugin-manager implementation around the reported lines,
# plus nearby timeout/limit behavior if present.
if [ -f src/main/plugin-manager.ts ]; then
  printf '--- plugin-manager.ts outline ---\n'
  ast-grep outline src/main/plugin-manager.ts || true
  printf '\n--- lines 60-110 ---\n'
  sed -n '60,110p' src/main/plugin-manager.ts | nl -ba -v60
  printf '\n--- all fetch/timeout/limit references ---\n'
  rg -n "fetch\\(|arrayBuffer|timeout|AbortController|writeFile|Response|Readable|createWriteStream|max|size|limit" src/main/plugin-manager.ts || true
else
  printf 'src/main/plugin-manager.ts not found\n'
  git ls-files | rg 'plugin-manager\.ts$' || true
fi

Repository: got-feedBack/feedBack-desktop

Length of output: 258

🏁 Script executed:

#!/bin/bash
set -euo pipefail

if [ -f src/main/plugin-manager.ts ]; then
  echo '--- plugin-manager.ts outline ---'
  ast-grep outline src/main/plugin-manager.ts || true
  echo
  echo '--- lines 60-110 ---'
  sed -n '60,110p' src/main/plugin-manager.ts | nl -ba -v60
  echo
  echo '--- fetch/timeout/limit references ---'
  rg -n "fetch\\(|arrayBuffer|timeout|AbortController|writeFile|Response|Readable|createWriteStream|max|size|limit" src/main/plugin-manager.ts || true
else
  echo 'src/main/plugin-manager.ts not found'
  git ls-files | rg 'plugin-manager\.ts$' || true
fi

Repository: got-feedBack/feedBack-desktop

Length of output: 1262

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- lines 78-95 ---'
sed -n '78,95p' src/main/plugin-manager.ts | awk '{print NR+77": "$0}'

echo
echo '--- fetch/timeout/limit references ---'
rg -n "fetch\\(|arrayBuffer|timeout|AbortController|writeFile|Response|Readable|createWriteStream|max|size|limit|archivePath" src/main/plugin-manager.ts || true

echo
echo '--- package runtime hints ---'
grep -n '"type":' package.json 2>/dev/null || true
sed -n '1,120p' src/main/plugin-manager.ts | awk '{print NR": "$0}'

Repository: got-feedBack/feedBack-desktop

Length of output: 7702

🌐 Web query:

Node.js global fetch default timeout no abort controller arrayBuffer buffers entire response

💡 Result:

In Node.js, the global fetch API—powered by undici—has a default timeout of 300 seconds [1][2][3]. This applies to both headers and the body, and is not a per-request configurable timeout in the traditional sense, but rather a default configuration within the undici dispatcher [1][2]. There is no global setting to change this default; to enforce a shorter timeout, the standard and recommended approach is to use AbortSignal.timeout passed within the fetch options [4][5][3]. Regarding arrayBuffer, calling response.arrayBuffer does indeed buffer the entire response body into memory [6]. The method is designed to read the response stream to completion and resolve with an ArrayBuffer containing the full data [6]. Because this process consumes the entire body into memory, it is not suitable for large payloads where memory efficiency is a concern; for large data, using the response body as a stream is the recommended approach [7][8].

Citations:

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- updatePlugin installGitHubArchive call sites ---'
sed -n '220,265p' src/main/plugin-manager.ts | awk '{print NR+220": "$0}'

echo
echo '--- installPlugin call site ---'
sed -n '152,200p' src/main/plugin-manager.ts | awk '{print NR+152": "$0}'

Repository: got-feedBack/feedBack-desktop

Length of output: 4792

Bound archive downloads.

installGitHubArchive() uses response.arrayBuffer() without any size limit and does not pass an AbortSignal, so stalled GitHub downloads can hang and large archives can block memory before being written to disk. Stream the archive to a temp file with a configurable byte limit and an abort timeout.

🧰 Tools
🪛 ast-grep (0.45.0)

[warning] 86-86: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(archivePath, Buffer.from(await response.arrayBuffer()))
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename-typescript)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/plugin-manager.ts` around lines 85 - 87, Update
installGitHubArchive() to stream the GitHub response directly into a temporary
file instead of buffering response.arrayBuffer() in memory. Enforce a
configurable maximum byte limit while streaming, and use an AbortSignal with a
timeout to terminate stalled downloads; preserve the existing HTTP status
validation and archive installation flow.

fs.mkdirSync(targetDir, { recursive: true });
await execFileAsync('tar', ['-xzf', archivePath, '--strip-components=1', '-C', targetDir]);
fs.writeFileSync(path.join(targetDir, PLUGIN_SOURCE_FILE), JSON.stringify({ url: gitUrl }) + '\n');
} finally {
fs.rmSync(tempDir, { recursive: true, force: true });
}
}

async function listInstalledPlugins(): Promise<InstalledPlugin[]> {
const pluginsDir = getPluginsDir();
const plugins: InstalledPlugin[] = [];
Expand All @@ -83,6 +117,7 @@ async function listInstalledPlugins(): Promise<InstalledPlugin[]> {
if (!isDir) continue;
const manifestPath = path.join(pluginPath, 'plugin.json');
const gitDir = path.join(pluginPath, '.git');
const archiveSource = readArchiveSource(pluginPath);

let manifest = null;
try {
Expand All @@ -105,6 +140,7 @@ async function listInstalledPlugins(): Promise<InstalledPlugin[]> {
name: entry.name,
path: pluginPath,
hasGit: fs.existsSync(gitDir),
canUpdate: fs.existsSync(gitDir) || archiveSource !== null,
manifest,
version,
});
Expand Down Expand Up @@ -149,6 +185,15 @@ async function installPlugin(gitUrl: string, name?: string): Promise<{ success:
} catch (e: any) {
// Clean up failed clone
try { fs.rmSync(targetDir, { recursive: true }); } catch { /* ignore */ }
if (e?.code === 'ENOENT' && parseGitHubRepository(gitUrl)) {
try {
await installGitHubArchive(gitUrl, targetDir);
return { success: true, message: `Installed "${name}" successfully. Restart to activate.` };
} catch (archiveError: any) {
try { fs.rmSync(targetDir, { recursive: true }); } catch { /* ignore */ }
return { success: false, message: `Git is unavailable and the GitHub download failed: ${archiveError.message}` };
}
}
return { success: false, message: `Failed to clone: ${e.message}` };
}
}
Expand Down Expand Up @@ -183,14 +228,34 @@ async function updatePlugin(name: string): Promise<{ success: boolean; message:
return { success: false, message: `Plugin "${name}" not found` };
}

if (!fs.existsSync(path.join(targetDir, '.git'))) {
if (!fs.existsSync(path.join(targetDir, '.git')) && !readArchiveSource(targetDir)) {
return { success: false, message: `Plugin "${name}" is not a git repository — cannot update` };
}

try {
const output = await execFileAsync('git', ['pull'], targetDir);
if (output.includes('Already up to date')) {
return { success: true, message: `"${name}" is already up to date` };
if (fs.existsSync(path.join(targetDir, '.git'))) {
const output = await execFileAsync('git', ['pull'], targetDir);
if (output.includes('Already up to date')) {
return { success: true, message: `"${name}" is already up to date` };
}
} else {
const sourceUrl = readArchiveSource(targetDir)!;
const suffix = `${process.pid}-${Date.now()}`;
const stagedDir = `${targetDir}.update-${suffix}`;
const backupDir = `${targetDir}.backup-${suffix}`;
try {
await installGitHubArchive(sourceUrl, stagedDir);
fs.renameSync(targetDir, backupDir);
try {
fs.renameSync(stagedDir, targetDir);
} catch (error) {
fs.renameSync(backupDir, targetDir);
throw error;
}
fs.rmSync(backupDir, { recursive: true, force: true });
} finally {
fs.rmSync(stagedDir, { recursive: true, force: true });
}
}
return { success: true, message: `Updated "${name}". Restart to activate changes.` };
} catch (e: any) {
Expand Down
15 changes: 15 additions & 0 deletions src/main/plugin-source.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
export const PLUGIN_SOURCE_FILE = '.feedback-plugin-source.json';

export function parseGitHubRepository(value: string): { owner: string; repo: string } | null {
try {
const url = new URL(value);
const parts = url.pathname.replace(/^\/+|\/+$/g, '').split('/');
if (url.protocol !== 'https:' || url.hostname.toLowerCase() !== 'github.com' || parts.length !== 2) return null;
const owner = parts[0];
const repo = parts[1].replace(/\.git$/, '');
if (!/^[A-Za-z0-9_.-]+$/.test(owner) || !/^[A-Za-z0-9_.-]+$/.test(repo)) return null;
Comment on lines +3 to +10

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Reject URLs containing credentials.

URL accepts https://token@github.com/owner/repo; the archive flow then writes that raw URL to .feedback-plugin-source.json, persisting a token/password in plaintext. Reject non-empty url.username or url.password, and add a regression test.

Proposed fix 
-        if (url.protocol !== 'https:' || url.hostname.toLowerCase() !== 'github.com' || parts.length !== 2) return null;
+        if (url.protocol !== 'https:' || url.hostname.toLowerCase() !== 'github.com' ||
+            url.username || url.password || parts.length !== 2) return null;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function parseGitHubRepository(value: string): { owner: string; repo: string } | null {
try {
const url = new URL(value);
const parts = url.pathname.replace(/^\/+|\/+$/g, '').split('/');
if (url.protocol !== 'https:' || url.hostname.toLowerCase() !== 'github.com' || parts.length !== 2) return null;
const owner = parts[0];
const repo = parts[1].replace(/\.git$/, '');
if (!/^[A-Za-z0-9_.-]+$/.test(owner) || !/^[A-Za-z0-9_.-]+$/.test(repo)) return null;
export function parseGitHubRepository(value: string): { owner: string; repo: string } | null {
try {
const url = new URL(value);
const parts = url.pathname.replace(/^\/+|\/+$/g, '').split('/');
if (url.protocol !== 'https:' || url.hostname.toLowerCase() !== 'github.com' ||
url.username || url.password || parts.length !== 2) return null;
const owner = parts[0];
const repo = parts[1].replace(/\.git$/, '');
if (!/^[A-Za-z0-9_.-]+$/.test(owner) || !/^[A-Za-z0-9_.-]+$/.test(repo)) return null;
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/plugin-source.ts` around lines 3 - 10, The parseGitHubRepository
function must reject GitHub URLs containing credentials before accepting the
repository path. Validate that url.username and url.password are both empty,
return null otherwise, and add a regression test covering a credential-bearing
URL to ensure it is not persisted or accepted.

return { owner, repo };
} catch {
return null;
}
}
2 changes: 1 addition & 1 deletion src/renderer/plugin-manager/screen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
<div class="text-xs text-slate-500 mt-0.5">v${version}</div>
</div>
<div class="flex gap-2">
${plugin.hasGit ? `<button class="pm-update text-xs px-2 py-1 rounded bg-blue-600 hover:bg-blue-500" data-name="${plugin.name}">Update</button>` : ''}
${plugin.canUpdate ? `<button class="pm-update text-xs px-2 py-1 rounded bg-blue-600 hover:bg-blue-500" data-name="${plugin.name}">Update</button>` : ''}
<button class="pm-remove text-xs px-2 py-1 rounded bg-red-600/50 hover:bg-red-500" data-name="${plugin.name}">Remove</button>
</div>
`;
Expand Down
17 changes: 17 additions & 0 deletions tests/plugin-source.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
const test = require('node:test');
const assert = require('node:assert/strict');
const { loadTs } = require('./_load-ts');

const { parseGitHubRepository } = loadTs('src/main/plugin-source.ts');

test('recognizes public GitHub repository URLs used by archive installs', () => {
assert.deepEqual(parseGitHubRepository('https://github.com/balki97/feedforge-connect'), {
owner: 'balki97', repo: 'feedforge-connect',
});
assert.deepEqual(parseGitHubRepository('https://github.com/balki97/feedforge-connect.git'), {
owner: 'balki97', repo: 'feedforge-connect',
});
assert.equal(parseGitHubRepository('https://example.com/balki97/feedforge-connect'), null);
assert.equal(parseGitHubRepository('file:///tmp/feedforge-connect'), null);
assert.equal(parseGitHubRepository('https://github.com/balki97/feedforge-connect/releases'), null);
});