mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-02 03:35:09 +02:00
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 <garagon@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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 {}
|
||||
|
||||
@@ -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=");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user