mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-13 16:03:04 +08:00
feat(resolver): parameterized LEARNINGS_SEARCH with shell-injection guard
The {{LEARNINGS_SEARCH}} macro now accepts a query=KEYWORD argument that
gets interpolated as --query "<keyword>" into the generated bash. Empty
value falls through to no-query (principle of least surprise: a stray
{{LEARNINGS_SEARCH:query=}} placeholder gets today's behavior, not a
build failure). Pattern reuses the parameterized-macro parsing from
composition.ts. The 13 templates that don't pass a query stay
byte-identical in their generated SKILL.md output.
Shell-injection guard: the query value is whitelisted to
^[A-Za-z0-9 _-]+$ at gen-skill-docs time. Any \$(), backticks,
semicolons, or quotes throw a loud build error instead of emitting
executable bash. Static template queries are safe by inspection;
this defends against future contributors writing dangerous values.
Adds 5 assertions to test/gen-skill-docs.test.ts covering no-args,
claude+query=foo bar on both cross-project and project-scoped branches,
codex host variant, empty value semantics, and shell-injection payloads
(\$(whoami), backticks, ;, &, ", \\, \$x) throwing build errors.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -13,7 +13,27 @@
|
||||
*/
|
||||
import type { TemplateContext } from './types';
|
||||
|
||||
export function generateLearningsSearch(ctx: TemplateContext): string {
|
||||
// Whitelist for query= macro values. Allows alphanumeric, space, hyphen, underscore.
|
||||
// Anything else (e.g. $, backticks, quotes, ;) is a shell-injection vector when the
|
||||
// emitted bash interpolates the value into `--query "${queryArg}"`. Static template
|
||||
// queries hand-written in gstack are safe, but the resolver API must defend against
|
||||
// future contributors writing dangerous values.
|
||||
const QUERY_SAFE_RE = /^[A-Za-z0-9 _-]+$/;
|
||||
|
||||
export function generateLearningsSearch(ctx: TemplateContext, args?: string[]): string {
|
||||
// Parse query= arg. Empty value falls through to no-query (principle of least surprise:
|
||||
// a stray {{LEARNINGS_SEARCH:query=}} placeholder gets today's behavior, not a build error).
|
||||
const queryArg = (args || [])
|
||||
.filter(a => a.startsWith('query='))
|
||||
.map(a => a.slice(6))
|
||||
.filter(Boolean)[0];
|
||||
if (queryArg && !QUERY_SAFE_RE.test(queryArg)) {
|
||||
throw new Error(
|
||||
`{{LEARNINGS_SEARCH:query=...}} value must match ${QUERY_SAFE_RE} (alphanumeric, space, hyphen, underscore). Got: ${JSON.stringify(queryArg)}`
|
||||
);
|
||||
}
|
||||
const queryFlag = queryArg ? ` --query "${queryArg}"` : '';
|
||||
|
||||
if (ctx.host === 'codex') {
|
||||
// Codex: simpler version, no cross-project, uses $GSTACK_BIN
|
||||
return `## Prior Learnings
|
||||
@@ -21,7 +41,7 @@ export function generateLearningsSearch(ctx: TemplateContext): string {
|
||||
Search for relevant learnings from previous sessions on this project:
|
||||
|
||||
\`\`\`bash
|
||||
$GSTACK_BIN/gstack-learnings-search --limit 10 2>/dev/null || true
|
||||
$GSTACK_BIN/gstack-learnings-search --limit 10${queryFlag} 2>/dev/null || true
|
||||
\`\`\`
|
||||
|
||||
If learnings are found, incorporate them into your analysis. When a review finding
|
||||
@@ -36,9 +56,9 @@ Search for relevant learnings from previous sessions:
|
||||
_CROSS_PROJ=$(${ctx.paths.binDir}/gstack-config get cross_project_learnings 2>/dev/null || echo "unset")
|
||||
echo "CROSS_PROJECT: $_CROSS_PROJ"
|
||||
if [ "$_CROSS_PROJ" = "true" ]; then
|
||||
${ctx.paths.binDir}/gstack-learnings-search --limit 10 --cross-project 2>/dev/null || true
|
||||
${ctx.paths.binDir}/gstack-learnings-search --limit 10${queryFlag} --cross-project 2>/dev/null || true
|
||||
else
|
||||
${ctx.paths.binDir}/gstack-learnings-search --limit 10 2>/dev/null || true
|
||||
${ctx.paths.binDir}/gstack-learnings-search --limit 10${queryFlag} 2>/dev/null || true
|
||||
fi
|
||||
\`\`\`
|
||||
|
||||
|
||||
@@ -3047,3 +3047,51 @@ describe('GSTACK REVIEW REPORT delete-then-append flow', () => {
|
||||
expect(src).not.toContain('If it was found mid-file, move it');
|
||||
});
|
||||
});
|
||||
|
||||
describe('LEARNINGS_SEARCH resolver: query parameter', () => {
|
||||
// Lazy-load resolver and types after describe block to keep test file self-contained.
|
||||
const { generateLearningsSearch } = require('../scripts/resolvers/learnings');
|
||||
const { HOST_PATHS } = require('../scripts/resolvers/types');
|
||||
|
||||
const claudeCtx = {
|
||||
skillName: 'test',
|
||||
tmplPath: 'test/SKILL.md.tmpl',
|
||||
host: 'claude',
|
||||
paths: HOST_PATHS.claude,
|
||||
};
|
||||
const codexCtx = { ...claudeCtx, host: 'codex', paths: HOST_PATHS.codex };
|
||||
|
||||
test('no args → bash does not contain --query (backwards-compat)', () => {
|
||||
const out = generateLearningsSearch(claudeCtx);
|
||||
expect(out).not.toContain('--query');
|
||||
});
|
||||
|
||||
test('claude host + query=foo bar → both cross-project and project-scoped branches contain --query', () => {
|
||||
const out = generateLearningsSearch(claudeCtx, ['query=foo bar']);
|
||||
// Both branches of the if/else must carry the flag.
|
||||
const lines = out.split('\n').filter(l => l.includes('gstack-learnings-search'));
|
||||
expect(lines.length).toBeGreaterThanOrEqual(2);
|
||||
for (const line of lines) {
|
||||
expect(line).toContain('--query "foo bar"');
|
||||
}
|
||||
});
|
||||
|
||||
test('codex host + query=foo bar → codex bash variant contains --query', () => {
|
||||
const out = generateLearningsSearch(codexCtx, ['query=foo bar']);
|
||||
expect(out).toContain('--query "foo bar"');
|
||||
expect(out).toContain('$GSTACK_BIN/gstack-learnings-search');
|
||||
});
|
||||
|
||||
test('empty value query= → bash does not contain --query (locked semantics: falls through)', () => {
|
||||
const claudeOut = generateLearningsSearch(claudeCtx, ['query=']);
|
||||
expect(claudeOut).not.toContain('--query');
|
||||
const codexOut = generateLearningsSearch(codexCtx, ['query=']);
|
||||
expect(codexOut).not.toContain('--query');
|
||||
});
|
||||
|
||||
test('shell-injection chars in query= → throws at gen-time (defense in depth)', () => {
|
||||
for (const bad of ['$(whoami)', '`cmd`', 'a;b', 'a&b', 'a"b', 'a\\b', 'foo$x']) {
|
||||
expect(() => generateLearningsSearch(claudeCtx, [`query=${bad}`])).toThrow(/alphanumeric/);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user