diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index f235d09..60f9a0c 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -14,10 +14,12 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 + with: + persist-credentials: false - uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22.14.0 registry-url: https://registry.npmjs.org - run: npm ci diff --git a/package-lock.json b/package-lock.json index a57a76b..02cecb6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@open-gitagent/gitagent", - "version": "2.0.1", + "version": "2.0.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@open-gitagent/gitagent", - "version": "2.0.1", + "version": "2.0.3", "license": "MIT", "dependencies": { "@mariozechner/pi-agent-core": "^0.70.2", @@ -1863,9 +1863,9 @@ "license": "MIT" }, "node_modules/basic-ftp": { - "version": "5.3.1", - "resolved": "https://registry.npmjs.org/basic-ftp/-/basic-ftp-5.3.1.tgz", - "integrity": "sha512-bopVNp6ugyA150DDuZfPFdt1KZ5a94ZDiwX4hMgZDzF+GttD80lEy8kj98kbyhLXnPvhtIo93mdnLIjpCAeeOw==", + "version": "6.0.1", + "resolved": "https://registry.npmjs.org/basic-ftp/-/basic-ftp-6.0.1.tgz", + "integrity": "sha512-3ilxa3n4276wGQp/ImRAuz4ALdsj/2Wd3FqoZBZlajDYnByCZ0JMb4+26Rde0wGXIbM0G2HWSfr/Fi8b21KX8g==", "license": "MIT", "engines": { "node": ">=10.0.0" @@ -2065,9 +2065,9 @@ "license": "MIT" }, "node_modules/fast-xml-builder": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/fast-xml-builder/-/fast-xml-builder-1.2.0.tgz", - "integrity": "sha512-00aAWieqff+ZJhsXA4g1g7M8k+7AYoMUUHF+/zFb5U6Uv/P0Vl4QZo84/IcufzYalLuEj9928bXN9PbbFzMF0Q==", + "version": "1.2.1", + "resolved": "https://registry.npmjs.org/fast-xml-builder/-/fast-xml-builder-1.2.1.tgz", + "integrity": "sha512-tPb5TTWfgfVx5BNSi2xV0eLr89POeXXn0dXIsCJ9m1narrWxeIyx6je9d7Rce/3NyXLbvuQmLkxq+RuxMWejvw==", "funding": [ { "type": "github", @@ -2759,13 +2759,16 @@ "license": "MIT" }, "node_modules/uuid": { - "version": "8.3.2", - "resolved": "https://registry.npmjs.org/uuid/-/uuid-8.3.2.tgz", - "integrity": "sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg==", - "deprecated": "uuid@10 and below is no longer supported. For ESM codebases, update to uuid@latest. For CommonJS codebases, use uuid@11 (but be aware this version will likely be deprecated in 2028).", + "version": "11.1.1", + "resolved": "https://registry.npmjs.org/uuid/-/uuid-11.1.1.tgz", + "integrity": "sha512-vIYxrBCC/N/K+Js3qSN88go7kIfNPssr/hHCesKCQNAjmgvYS2oqr69kIufEG+O4+PfezOH4EbIeHCfFov8ZgQ==", + "funding": [ + "https://github.com/sponsors/broofa", + "https://github.com/sponsors/ctavan" + ], "license": "MIT", "bin": { - "uuid": "dist/bin/uuid" + "uuid": "dist/esm/bin/uuid" } }, "node_modules/web-streams-polyfill": { diff --git a/package.json b/package.json index 1e12d2e..e6138c9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@open-gitagent/gitagent", - "version": "2.0.1", + "version": "2.0.3", "description": "A universal git-native multimodal always learning AI Agent (TinyHuman)", "author": "shreyaskapale", "license": "MIT", @@ -79,5 +79,9 @@ "@types/node": "^22.0.0", "@types/node-cron": "^3.0.11", "typescript": "^5.7.0" + }, + "overrides": { + "basic-ftp": "^6.0.1", + "uuid": "^11.1.1" } } diff --git a/src/chat-history.ts b/src/chat-history.ts index 36f98f8..fa04b53 100644 --- a/src/chat-history.ts +++ b/src/chat-history.ts @@ -6,8 +6,11 @@ import { query } from "./sdk.js"; /** Types we skip — too large or ephemeral */ const SKIP_TYPES = new Set(["audio_delta", "agent_thinking"]); -function sanitizeBranch(branch: string): string { - return branch.replace(/\//g, "__"); +export function sanitizeBranch(branch: string): string { + // Allowlist safe filename characters — anything else becomes "__". This + // can never yield a path separator ("/" or "\") or a "." segment, so the + // result can never traverse outside historyDir, and it is trivial to audit. + return branch.replace(/[^a-zA-Z0-9_-]/g, "__"); } function historyDir(agentDir: string): string { diff --git a/src/config.ts b/src/config.ts index c130cfd..01e7961 100644 --- a/src/config.ts +++ b/src/config.ts @@ -8,9 +8,12 @@ export interface EnvConfig { [key: string]: any; } +const UNSAFE_MERGE_KEYS = new Set(["__proto__", "constructor", "prototype"]); + function deepMerge(base: Record, override: Record): Record { const result = { ...base }; for (const key of Object.keys(override)) { + if (UNSAFE_MERGE_KEYS.has(key)) continue; if ( result[key] && typeof result[key] === "object" && @@ -47,6 +50,9 @@ export async function loadEnvConfig(agentDir: string, env?: string): Promise const available: KnowledgeEntry[] = []; for (const entry of index.entries) { + // Reject any entry whose path escapes knowledgeDir — applied to both + // always_load and on-demand (available) entries, so a malicious + // index.yaml can't be surfaced for later reads either. + const resolvedPath = resolve(join(knowledgeDir, entry.path)); + const rel = relative(resolve(knowledgeDir), resolvedPath); + if (rel === ".." || rel.startsWith(".." + sep) || isAbsolute(rel)) { + continue; // entry.path escapes knowledgeDir — skip + } if (entry.always_load) { try { - const content = await readFile(join(knowledgeDir, entry.path), "utf-8"); + const content = await readFile(resolvedPath, "utf-8"); preloaded.push({ path: entry.path, content: content.trim() }); } catch { // Skip missing files diff --git a/src/loader.ts b/src/loader.ts index b8d193e..7d67381 100644 --- a/src/loader.ts +++ b/src/loader.ts @@ -1,7 +1,7 @@ import { readFile, mkdir, writeFile } from "fs/promises"; import { join } from "path"; import { randomUUID } from "crypto"; -import { execSync } from "child_process"; +import { execFileSync } from "child_process"; import { getModel } from "@mariozechner/pi-ai"; import type { Model } from "@mariozechner/pi-ai"; import yaml from "js-yaml"; @@ -142,9 +142,12 @@ export interface LoadedAgent { plugins: LoadedPlugin[]; } +const UNSAFE_MERGE_KEYS = new Set(["__proto__", "constructor", "prototype"]); + function deepMerge(base: Record, override: Record): Record { const result = { ...base }; for (const key of Object.keys(override)) { + if (UNSAFE_MERGE_KEYS.has(key)) continue; if ( result[key] && typeof result[key] === "object" && @@ -173,11 +176,14 @@ async function resolveInheritance( await mkdir(depsDir, { recursive: true }); // Clone parent into .gitagent/deps/ - const parentName = manifest.extends.split("/").pop()?.replace(/\.git$/, "") || "parent"; + const rawParentName = manifest.extends.split("/").pop()?.replace(/\.git$/, "") || "parent"; + // Strip anything that isn't a safe identifier char — handles backslashes, + // "..", etc. that could otherwise survive the forward-slash-only split above. + const parentName = rawParentName.replace(/[^a-zA-Z0-9_-]/g, "_") || "parent"; const parentDir = join(depsDir, parentName); try { - execSync(`git clone --depth 1 "${manifest.extends}" "${parentDir}" 2>/dev/null || true`, { + execFileSync("git", ["clone", "--depth", "1", manifest.extends, parentDir], { cwd: agentDir, stdio: "pipe", }); @@ -221,10 +227,15 @@ async function resolveDependencies( await mkdir(depsDir, { recursive: true }); for (const dep of manifest.dependencies) { + if (!/^[a-zA-Z0-9_-]+$/.test(dep.name)) { + console.warn(`Skipping dependency with invalid name "${dep.name}" — only letters, digits, "-", and "_" are allowed`); + continue; + } const depDir = join(depsDir, dep.name); try { - execSync( - `git clone --depth 1 --branch "${dep.version}" "${dep.source}" "${depDir}" 2>/dev/null || true`, + execFileSync( + "git", + ["clone", "--depth", "1", "--branch", dep.version, dep.source, depDir], { cwd: agentDir, stdio: "pipe" }, ); } catch { diff --git a/src/plugin-cli.ts b/src/plugin-cli.ts index 50927bb..96a8dbe 100644 --- a/src/plugin-cli.ts +++ b/src/plugin-cli.ts @@ -135,6 +135,10 @@ async function handleRemove(agentDir: string, args: string[]): Promise { console.error(red("Usage: gitagent plugin remove ")); process.exit(1); } + if (!/^[a-z0-9]+(-[a-z0-9]+)*$/.test(name)) { + console.error(red("Plugin name must be kebab-case (e.g., my-plugin)")); + process.exit(1); + } // Try local first, then installed const localDir = join(agentDir, "plugins", name); diff --git a/src/plugins.ts b/src/plugins.ts index 8a22899..1ec7782 100644 --- a/src/plugins.ts +++ b/src/plugins.ts @@ -1,5 +1,5 @@ import { readFile, readdir, stat, mkdir, rm } from "fs/promises"; -import { join } from "path"; +import { join, resolve, relative, isAbsolute, sep } from "path"; import { execFileSync } from "child_process"; import { createRequire } from "module"; import { homedir } from "os"; @@ -19,6 +19,16 @@ import type { SkillMetadata } from "./skills.js"; const require = createRequire(import.meta.url); const { version: GITAGENT_VERSION } = require("../package.json"); +// Resolves relPath against baseDir and returns the result only if it stays +// within baseDir — guards against a plugin manifest (author-controlled, +// untrusted for third-party plugins) pointing outside its own directory. +function resolveWithinDir(baseDir: string, relPath: string): string | null { + const resolvedPath = resolve(join(baseDir, relPath)); + const rel = relative(resolve(baseDir), resolvedPath); + if (rel === ".." || rel.startsWith(".." + sep) || isAbsolute(rel)) return null; + return resolvedPath; +} + const KEBAB_RE = /^[a-z0-9]+(-[a-z0-9]+)*$/; // ── Engine version check ──────────────────────────────────────────────── @@ -134,7 +144,10 @@ export async function installPlugin( await mkdir(targetDir, { recursive: true }); // Derive plugin name from source - const name = source.split("/").pop()?.replace(/\.git$/, "") || "plugin"; + const rawName = source.split("/").pop()?.replace(/\.git$/, "") || "plugin"; + // Normalize to lowercase kebab-case so the derived name always satisfies + // KEBAB_RE at load time (avoids "installs but fails to load" mismatches). + const name = rawName.toLowerCase().replace(/[^a-z0-9-]/g, "-").replace(/-+/g, "-").replace(/^-|-$/g, "") || "plugin"; const pluginDir = join(targetDir, name); if (await dirExists(pluginDir)) { @@ -227,10 +240,15 @@ async function loadPlugin( // Load prompt addition let promptAddition = ""; if (manifest.provides?.prompt) { - try { - promptAddition = await readFile(join(pluginDir, manifest.provides.prompt), "utf-8"); - } catch { - // Prompt file not found, skip + const promptPath = resolveWithinDir(pluginDir, manifest.provides.prompt); + if (promptPath) { + try { + promptAddition = await readFile(promptPath, "utf-8"); + } catch { + // Prompt file not found, skip + } + } else { + console.warn(`Plugin "${manifest.id}": provides.prompt escapes plugin directory — skipping`); } } @@ -238,36 +256,40 @@ async function loadPlugin( let programmaticTools: any[] = []; let memoryLayers: import("./plugin-types.js").MemoryLayerDef[] = []; if (manifest.entry) { - try { - const { createPluginApi } = await import("./plugin-sdk.js"); - const api = createPluginApi(manifest.id, pluginDir, config); - const entryPath = join(pluginDir, manifest.entry); - const mod = await import(entryPath); - if (typeof mod.register === "function") { - await mod.register(api); - } else if (typeof mod.default === "function") { - await mod.default(api); - } - programmaticTools = api.getTools(); - // Merge programmatic hooks - const progHooks = api.getHooks(); - if (progHooks) { - if (!hooks) hooks = { hooks: {} }; - for (const event of ["on_session_start", "pre_tool_use", "post_response", "on_error"] as const) { - if (progHooks[event]) { - hooks.hooks[event] = [...(hooks.hooks[event] || []), ...progHooks[event]!]; + const entryPath = resolveWithinDir(pluginDir, manifest.entry); + if (!entryPath) { + console.warn(`Plugin "${manifest.id}": entry escapes plugin directory — skipping`); + } else { + try { + const { createPluginApi } = await import("./plugin-sdk.js"); + const api = createPluginApi(manifest.id, pluginDir, config); + const mod = await import(entryPath); + if (typeof mod.register === "function") { + await mod.register(api); + } else if (typeof mod.default === "function") { + await mod.default(api); + } + programmaticTools = api.getTools(); + // Merge programmatic hooks + const progHooks = api.getHooks(); + if (progHooks) { + if (!hooks) hooks = { hooks: {} }; + for (const event of ["on_session_start", "pre_tool_use", "post_response", "on_error"] as const) { + if (progHooks[event]) { + hooks.hooks[event] = [...(hooks.hooks[event] || []), ...progHooks[event]!]; + } } } + // Merge programmatic prompt + const extraPrompt = api.getPrompt(); + if (extraPrompt) { + promptAddition = promptAddition ? `${promptAddition}\n\n${extraPrompt}` : extraPrompt; + } + // Collect memory layers + memoryLayers = api.getMemoryLayers(); + } catch (err: any) { + console.warn(`Plugin "${manifest.id}": failed to load entry "${manifest.entry}": ${err.message}`); } - // Merge programmatic prompt - const extraPrompt = api.getPrompt(); - if (extraPrompt) { - promptAddition = promptAddition ? `${promptAddition}\n\n${extraPrompt}` : extraPrompt; - } - // Collect memory layers - memoryLayers = api.getMemoryLayers(); - } catch (err: any) { - console.warn(`Plugin "${manifest.id}": failed to load entry "${manifest.entry}": ${err.message}`); } } @@ -323,6 +345,10 @@ export async function discoverAndLoadPlugins( for (const [pluginName, pluginConf] of Object.entries(pluginsConfig)) { // Skip disabled plugins if (pluginConf.enabled === false) continue; + if (!/^[a-zA-Z0-9_-]+$/.test(pluginName)) { + console.warn(`Skipping plugin with invalid name "${pluginName}" — only letters, digits, "-", and "_" are allowed`); + continue; + } // Auto-install from source if needed if (pluginConf.source) { diff --git a/src/schedules.ts b/src/schedules.ts index 44b2970..6d59d9d 100644 --- a/src/schedules.ts +++ b/src/schedules.ts @@ -39,7 +39,7 @@ export async function discoverSchedules(agentDir: string): Promise; - if (data?.id && data?.prompt && (data?.cron || data?.runAt)) { + if (data?.id && KEBAB_RE.test(String(data.id)) && data?.prompt && (data?.cron || data?.runAt)) { schedules.push({ id: String(data.id), prompt: String(data.prompt), @@ -66,6 +66,9 @@ export async function loadSchedule(filePath: string): Promise { + if (!KEBAB_RE.test(id)) { + throw new Error("Schedule id must be kebab-case (e.g. daily-standup)"); + } const filePath = join(agentDir, "schedules", `${id}.yaml`); await unlink(filePath); } export async function updateScheduleMeta(agentDir: string, id: string, updates: Partial>): Promise { + if (!KEBAB_RE.test(id)) { + throw new Error("Schedule id must be kebab-case (e.g. daily-standup)"); + } const filePath = join(agentDir, "schedules", `${id}.yaml`); const raw = await readFile(filePath, "utf-8"); const data = yaml.load(raw) as Record; diff --git a/src/session.ts b/src/session.ts index cdc6a45..91906b7 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1,4 +1,4 @@ -import { execSync } from "child_process"; +import { execFileSync } from "child_process"; import { existsSync, mkdirSync, writeFileSync } from "fs"; import { resolve } from "path"; import { randomBytes } from "crypto"; @@ -32,19 +32,19 @@ function cleanUrl(url: string): string { return url.replace(/^https:\/\/[^@]+@/, "https://"); } -function git(args: string, cwd: string): string { - return execSync(`git ${args}`, { cwd, stdio: "pipe", encoding: "utf-8" }).trim(); +function git(args: string[], cwd: string): string { + return execFileSync("git", args, { cwd, stdio: "pipe", encoding: "utf-8" }).trim(); } function getDefaultBranch(cwd: string): string { try { // e.g. "origin/main" → "main" - const ref = git("symbolic-ref refs/remotes/origin/HEAD", cwd); + const ref = git(["symbolic-ref", "refs/remotes/origin/HEAD"], cwd); return ref.replace("refs/remotes/origin/", ""); } catch { // Fallback: try main, then master try { - git("rev-parse --verify origin/main", cwd); + git(["rev-parse", "--verify", "origin/main"], cwd); return "main"; } catch { return "master"; @@ -61,15 +61,15 @@ export function initLocalSession(opts: LocalRepoOptions): LocalSession { // Clone or update if (!existsSync(dir)) { - execSync(`git clone --depth 1 --no-single-branch ${aUrl} ${dir}`, { stdio: "pipe" }); + execFileSync("git", ["clone", "--depth", "1", "--no-single-branch", aUrl, dir], { stdio: "pipe" }); } else { - git(`remote set-url origin ${aUrl}`, dir); - git("fetch origin", dir); + git(["remote", "set-url", "origin", aUrl], dir); + git(["fetch", "origin"], dir); // Reset local default branch to latest remote const defaultBranch = getDefaultBranch(dir); - git(`checkout ${defaultBranch}`, dir); - git(`reset --hard origin/${defaultBranch}`, dir); + git(["checkout", defaultBranch], dir); + git(["reset", "--hard", `origin/${defaultBranch}`], dir); } // Determine branch @@ -83,17 +83,17 @@ export function initLocalSession(opts: LocalRepoOptions): LocalSession { // Try local checkout first, fall back to remote tracking try { - git(`checkout ${branch}`, dir); + git(["checkout", branch], dir); } catch { - git(`checkout -b ${branch} origin/${branch}`, dir); + git(["checkout", "-b", branch, `origin/${branch}`], dir); } // Pull latest for existing session branch - try { git(`pull origin ${branch}`, dir); } catch { /* branch may not exist on remote yet */ } + try { git(["pull", "origin", branch], dir); } catch { /* branch may not exist on remote yet */ } } else { // New session — branch off latest default branch sessionId = randomBytes(4).toString("hex"); // 8-char hex branch = `gitagent/session-${sessionId}`; - git(`checkout -b ${branch}`, dir); + git(["checkout", "-b", branch], dir); } // Scaffold agent.yaml + memory if missing (on session branch only) @@ -128,26 +128,26 @@ export function initLocalSession(opts: LocalRepoOptions): LocalSession { sessionId, commitChanges(msg?: string) { - git("add -A", dir); + git(["add", "-A"], dir); try { - git("diff --cached --quiet", dir); + git(["diff", "--cached", "--quiet"], dir); // Nothing staged — skip } catch { // There are staged changes const commitMsg = msg || `gitagent: auto-commit (${branch})`; - git(`commit -m "${commitMsg}"`, dir); + git(["commit", "-m", commitMsg], dir); } }, push() { - git(`push origin ${branch}`, dir); + git(["push", "origin", branch], dir); }, finalize() { localSession.commitChanges(); localSession.push(); // Strip PAT from remote URL - git(`remote set-url origin ${cleanUrl(url)}`, dir); + git(["remote", "set-url", "origin", cleanUrl(url)], dir); }, }; diff --git a/src/tool-loader.ts b/src/tool-loader.ts index 3ac4025..aa76b45 100644 --- a/src/tool-loader.ts +++ b/src/tool-loader.ts @@ -1,5 +1,5 @@ import { readFile, readdir, stat } from "fs/promises"; -import { join } from "path"; +import { join, resolve, relative, isAbsolute, sep } from "path"; import { spawn } from "child_process"; import yaml from "js-yaml"; import { Type } from "@sinclair/typebox"; @@ -47,13 +47,34 @@ export function buildTypeboxSchema(schema: Record): any { return Type.Object(properties); } +// Interpreters a declarative tool's implementation.runtime is allowed to name. +// spawn() below is already called in array form (no shell: true), so shell +// metacharacters in scriptPath/runtime are never interpreted — this allowlist +// is defense-in-depth against a tool config naming an unexpected binary/path. +const ALLOWED_RUNTIMES = new Set(["sh", "bash", "node", "python3", "python"]); + function createDeclarativeTool( def: ToolDefinition, agentDir: string, ): AgentTool { const schema = buildTypeboxSchema(def.input_schema); - const scriptPath = join(agentDir, "tools", def.implementation.script); + const toolsDir = join(agentDir, "tools"); + const scriptPath = join(toolsDir, def.implementation.script); + + // Path traversal guard: ensure script doesn't escape the tools/ directory + const resolvedScript = resolve(scriptPath); + const allowedBase = resolve(toolsDir); + const rel = relative(allowedBase, resolvedScript); + if (rel !== "" && (rel === ".." || rel.startsWith(".." + sep) || isAbsolute(rel))) { + throw new Error(`Tool "${def.name}" script "${def.implementation.script}" escapes the tools directory`); + } + const runtime = def.implementation.runtime || "sh"; + if (!ALLOWED_RUNTIMES.has(runtime)) { + throw new Error( + `Tool "${def.name}" specifies unsupported runtime "${runtime}". Allowed: ${[...ALLOWED_RUNTIMES].join(", ")}`, + ); + } return { name: def.name, diff --git a/src/tools/capture-photo.ts b/src/tools/capture-photo.ts index 2733fb7..d6cc6fc 100644 --- a/src/tools/capture-photo.ts +++ b/src/tools/capture-photo.ts @@ -1,6 +1,6 @@ import { readFile, writeFile, mkdir, stat } from "fs/promises"; import { join } from "path"; -import { execSync } from "child_process"; +import { execFileSync } from "child_process"; import type { AgentTool } from "@mariozechner/pi-agent-core"; import { capturePhotoSchema } from "./shared.js"; @@ -85,10 +85,8 @@ export function createCapturePhotoTool(cwd: string): AgentTool; - if (data?.name) { + if (data?.name && KEBAB_RE.test(String(data.name))) { const isFlow = Array.isArray(data.steps) && data.steps.length > 0; workflows.push({ name: data.name, @@ -137,6 +137,9 @@ export async function saveFlowDefinition(agentDir: string, flow: SkillFlowDefini } export async function deleteFlowDefinition(agentDir: string, name: string): Promise { + if (!KEBAB_RE.test(name)) { + throw new Error("Flow name must be kebab-case (e.g. my-flow-name)"); + } const filePath = join(agentDir, "workflows", `${name}.yaml`); await unlink(filePath); }