fix(manager): validate flag tokens to prevent injection via config (#1410)
Address review feedback: sanitize manager.flags values to allow only CLI-safe tokens (--flag patterns and alphanumeric values). Invalid tokens are dropped with a stderr warning. Prevents prompt injection via compromised config.json. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1028,10 +1028,23 @@ function cmdInitManager(cwd, raw) {
|
||||
const completedCount = phases.filter(p => p.disk_status === 'complete').length;
|
||||
|
||||
// Read manager flags from config (passthrough flags for each step)
|
||||
// Validate: flags must be CLI-safe (only --flags, alphanumeric, hyphens, spaces)
|
||||
const sanitizeFlags = (raw) => {
|
||||
const val = typeof raw === 'string' ? raw : '';
|
||||
if (!val) return '';
|
||||
// Allow only --flag patterns with alphanumeric/hyphen values separated by spaces
|
||||
const tokens = val.split(/\s+/).filter(Boolean);
|
||||
const safe = tokens.every(t => /^--[a-zA-Z0-9][-a-zA-Z0-9]*$/.test(t) || /^[a-zA-Z0-9][-a-zA-Z0-9_.]*$/.test(t));
|
||||
if (!safe) {
|
||||
process.stderr.write(`gsd-tools: warning: manager.flags contains invalid tokens, ignoring: ${val}\n`);
|
||||
return '';
|
||||
}
|
||||
return val;
|
||||
};
|
||||
const managerFlags = {
|
||||
discuss: (config.manager && config.manager.flags && config.manager.flags.discuss) || '',
|
||||
plan: (config.manager && config.manager.flags && config.manager.flags.plan) || '',
|
||||
execute: (config.manager && config.manager.flags && config.manager.flags.execute) || '',
|
||||
discuss: sanitizeFlags(config.manager && config.manager.flags && config.manager.flags.discuss),
|
||||
plan: sanitizeFlags(config.manager && config.manager.flags && config.manager.flags.plan),
|
||||
execute: sanitizeFlags(config.manager && config.manager.flags && config.manager.flags.execute),
|
||||
};
|
||||
|
||||
const result = {
|
||||
|
||||
@@ -453,4 +453,30 @@ describe('init manager', () => {
|
||||
assert.strictEqual(output.manager_flags.plan, '--skip-research');
|
||||
assert.strictEqual(output.manager_flags.execute, '--interactive');
|
||||
});
|
||||
|
||||
test('sanitizes invalid manager_flags to prevent injection (#1410)', () => {
|
||||
writeState(tmpDir);
|
||||
writeRoadmap(tmpDir, [{ number: '1', name: 'Test' }]);
|
||||
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'config.json'),
|
||||
JSON.stringify({
|
||||
manager: {
|
||||
flags: {
|
||||
discuss: '; rm -rf /',
|
||||
plan: '--valid-flag',
|
||||
execute: '$(whoami)',
|
||||
}
|
||||
}
|
||||
})
|
||||
);
|
||||
|
||||
const result = runGsdTools('init manager', tmpDir);
|
||||
const output = JSON.parse(result.output);
|
||||
|
||||
// Invalid flags should be sanitized to empty string
|
||||
assert.strictEqual(output.manager_flags.discuss, '', 'injection attempt should be sanitized');
|
||||
assert.strictEqual(output.manager_flags.plan, '--valid-flag', 'valid flag should pass through');
|
||||
assert.strictEqual(output.manager_flags.execute, '', 'command substitution should be sanitized');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user