From 4bc4bfedfdcd55bbd48af674d306a8aed9a9488d Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 5 Apr 2026 11:44:07 -0700 Subject: [PATCH] fix(bin): pass search params via env vars (RCE fix) (#819) Replace shell string interpolation with process.env in gstack-learnings-search to prevent arbitrary code execution via crafted learnings entries. Also fixes the CROSS_PROJECT interpolation that the original PR missed. Adds 3 regression tests verifying no shell interpolation remains in the bun -e block. Co-authored-by: garagon Co-Authored-By: Claude Opus 4.6 (1M context) --- bin/gstack-learnings-search | 13 +++++---- test/learnings-injection.test.ts | 48 ++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 6 deletions(-) create mode 100644 test/learnings-injection.test.ts diff --git a/bin/gstack-learnings-search b/bin/gstack-learnings-search index 4ac187ec..634342e6 100755 --- a/bin/gstack-learnings-search +++ b/bin/gstack-learnings-search @@ -43,13 +43,14 @@ if [ ${#FILES[@]} -eq 0 ]; then fi # Process all files through bun for JSON parsing, decay, dedup, filtering -cat "${FILES[@]}" 2>/dev/null | bun -e " +GSTACK_SEARCH_TYPE="$TYPE" GSTACK_SEARCH_QUERY="$QUERY" GSTACK_SEARCH_LIMIT="$LIMIT" GSTACK_SEARCH_SLUG="$SLUG" GSTACK_SEARCH_CROSS="$CROSS_PROJECT" \ +cat "${FILES[@]}" 2>/dev/null | GSTACK_SEARCH_TYPE="$TYPE" GSTACK_SEARCH_QUERY="$QUERY" GSTACK_SEARCH_LIMIT="$LIMIT" GSTACK_SEARCH_SLUG="$SLUG" GSTACK_SEARCH_CROSS="$CROSS_PROJECT" bun -e " const lines = (await Bun.stdin.text()).trim().split('\n').filter(Boolean); const now = Date.now(); -const type = '${TYPE}'; -const query = '${QUERY}'.toLowerCase(); -const limit = ${LIMIT}; -const slug = '${SLUG}'; +const type = process.env.GSTACK_SEARCH_TYPE || ''; +const query = (process.env.GSTACK_SEARCH_QUERY || '').toLowerCase(); +const limit = parseInt(process.env.GSTACK_SEARCH_LIMIT || '10', 10); +const slug = process.env.GSTACK_SEARCH_SLUG || ''; const entries = []; for (const line of lines) { @@ -67,7 +68,7 @@ for (const line of lines) { // Determine if this is from the current project or cross-project // Cross-project entries are tagged for display - e._crossProject = !line.includes(slug) && '${CROSS_PROJECT}' === 'true'; + e._crossProject = !line.includes(slug) && process.env.GSTACK_SEARCH_CROSS === 'true'; entries.push(e); } catch {} diff --git a/test/learnings-injection.test.ts b/test/learnings-injection.test.ts new file mode 100644 index 00000000..4a2af56b --- /dev/null +++ b/test/learnings-injection.test.ts @@ -0,0 +1,48 @@ +import { describe, test, expect } from "bun:test"; +import { readFileSync } from "fs"; +import path from "path"; + +const SCRIPT = path.join(import.meta.dir, "..", "bin", "gstack-learnings-search"); + +describe("gstack-learnings-search injection prevention", () => { + const script = readFileSync(SCRIPT, "utf-8"); + + test("no shell interpolation inside bun -e string", () => { + // Extract the bun -e block (everything between `bun -e "` and the closing `"`) + const bunBlock = script.slice(script.indexOf('bun -e "')); + + // Should NOT contain ${VAR} patterns (shell interpolation) + // These are RCE vectors: a malicious learnings entry with '; rm -rf / ;' in the + // query field would execute arbitrary commands via shell interpolation. + const shellInterpolations = bunBlock.match(/'\$\{[A-Z_]+\}'/g) || []; + const bareInterpolations = bunBlock.match(/\$\{[A-Z_]+\}/g) || []; + + // Filter out any that are inside process.env references (those are safe) + const unsafeInterpolations = [ + ...shellInterpolations, + ...bareInterpolations, + ].filter((m) => !m.includes("process.env")); + + expect(unsafeInterpolations).toEqual([]); + }); + + test("uses process.env for all user-controlled values", () => { + const bunBlock = script.slice(script.indexOf('bun -e "')); + + // Must use process.env for TYPE, QUERY, LIMIT, SLUG, CROSS_PROJECT + expect(bunBlock).toContain("process.env.GSTACK_SEARCH_TYPE"); + expect(bunBlock).toContain("process.env.GSTACK_SEARCH_QUERY"); + expect(bunBlock).toContain("process.env.GSTACK_SEARCH_LIMIT"); + expect(bunBlock).toContain("process.env.GSTACK_SEARCH_SLUG"); + expect(bunBlock).toContain("process.env.GSTACK_SEARCH_CROSS"); + }); + + test("env vars are set on the bun command line", () => { + // The env vars must be passed to bun, not just set in the shell + expect(script).toContain("GSTACK_SEARCH_TYPE="); + expect(script).toContain("GSTACK_SEARCH_QUERY="); + expect(script).toContain("GSTACK_SEARCH_LIMIT="); + expect(script).toContain("GSTACK_SEARCH_SLUG="); + expect(script).toContain("GSTACK_SEARCH_CROSS="); + }); +});