fix(openclaw): improve shell argument escaping (#288)
Add comprehensive shell argument escaping using POSIX single-quote method. Problem: - Current code only escapes single quotes inline - Other shell metacharacters ($, `, !, etc.) not explicitly handled - Document ID in retain() was not escaped Solution: - Add exported escapeShellArg() function using POSIX single-quote escaping - Replace inline escaping with shared function - Escape document ID in retain() - Add comprehensive tests (17 test cases) covering all shell-special chars The POSIX single-quote method handles ALL shell metacharacters by wrapping in single quotes (which protect everything except single quotes themselves) and escaping any embedded single quotes with '\'' sequence.
This commit is contained in:
parent
d5403a4b29
commit
63e2964a4c
2 changed files with 115 additions and 6 deletions
|
|
@ -1,5 +1,5 @@
|
|||
import { describe, it, expect } from 'vitest';
|
||||
import { HindsightClient } from './client.js';
|
||||
import { HindsightClient, escapeShellArg } from './client.js';
|
||||
|
||||
describe('HindsightClient', () => {
|
||||
it('should create instance with provider and API key', () => {
|
||||
|
|
@ -21,3 +21,84 @@ describe('HindsightClient', () => {
|
|||
expect(client).toBeDefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('escapeShellArg', () => {
|
||||
it('should return unchanged string when no special characters', () => {
|
||||
expect(escapeShellArg('hello world')).toBe('hello world');
|
||||
expect(escapeShellArg('simple text 123')).toBe('simple text 123');
|
||||
});
|
||||
|
||||
it('should escape single quotes', () => {
|
||||
expect(escapeShellArg("it's")).toBe("it'\\''s");
|
||||
expect(escapeShellArg("don't")).toBe("don'\\''t");
|
||||
expect(escapeShellArg("'quoted'")).toBe("'\\''quoted'\\''");
|
||||
});
|
||||
|
||||
it('should preserve dollar signs (protected by single quotes)', () => {
|
||||
// These are NOT escaped - single quotes protect them
|
||||
expect(escapeShellArg('$HOME')).toBe('$HOME');
|
||||
expect(escapeShellArg('cost is $100')).toBe('cost is $100');
|
||||
});
|
||||
|
||||
it('should preserve backticks (protected by single quotes)', () => {
|
||||
expect(escapeShellArg('`ls`')).toBe('`ls`');
|
||||
expect(escapeShellArg('run `command`')).toBe('run `command`');
|
||||
});
|
||||
|
||||
it('should preserve exclamation marks (protected by single quotes)', () => {
|
||||
expect(escapeShellArg('hello!')).toBe('hello!');
|
||||
expect(escapeShellArg('wow! amazing!')).toBe('wow! amazing!');
|
||||
});
|
||||
|
||||
it('should preserve glob patterns (protected by single quotes)', () => {
|
||||
expect(escapeShellArg('*.txt')).toBe('*.txt');
|
||||
expect(escapeShellArg('file?.log')).toBe('file?.log');
|
||||
expect(escapeShellArg('[abc]')).toBe('[abc]');
|
||||
});
|
||||
|
||||
it('should preserve parentheses and braces (protected by single quotes)', () => {
|
||||
expect(escapeShellArg('(subshell)')).toBe('(subshell)');
|
||||
expect(escapeShellArg('{a,b,c}')).toBe('{a,b,c}');
|
||||
});
|
||||
|
||||
it('should preserve redirection and control operators (protected by single quotes)', () => {
|
||||
expect(escapeShellArg('a > b')).toBe('a > b');
|
||||
expect(escapeShellArg('cmd | grep')).toBe('cmd | grep');
|
||||
expect(escapeShellArg('a && b')).toBe('a && b');
|
||||
expect(escapeShellArg('a; b')).toBe('a; b');
|
||||
});
|
||||
|
||||
it('should preserve backslashes (protected by single quotes)', () => {
|
||||
expect(escapeShellArg('path\\to\\file')).toBe('path\\to\\file');
|
||||
});
|
||||
|
||||
it('should preserve double quotes (protected by single quotes)', () => {
|
||||
expect(escapeShellArg('"quoted"')).toBe('"quoted"');
|
||||
});
|
||||
|
||||
it('should preserve hash (protected by single quotes)', () => {
|
||||
expect(escapeShellArg('# comment')).toBe('# comment');
|
||||
});
|
||||
|
||||
it('should preserve tilde (protected by single quotes)', () => {
|
||||
expect(escapeShellArg('~user')).toBe('~user');
|
||||
});
|
||||
|
||||
it('should preserve newlines (protected by single quotes)', () => {
|
||||
expect(escapeShellArg('line1\nline2')).toBe('line1\nline2');
|
||||
});
|
||||
|
||||
it('should handle complex mixed content', () => {
|
||||
expect(escapeShellArg("It's $100! Run `ls`")).toBe("It'\\''s $100! Run `ls`");
|
||||
expect(escapeShellArg("user's file*.txt")).toBe("user'\\''s file*.txt");
|
||||
});
|
||||
|
||||
it('should handle empty string', () => {
|
||||
expect(escapeShellArg('')).toBe('');
|
||||
});
|
||||
|
||||
it('should handle multiple consecutive single quotes', () => {
|
||||
expect(escapeShellArg("''")).toBe("'\\'''\\''");
|
||||
expect(escapeShellArg("'''")).toBe("'\\'''\\'''\\''");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,4 +1,3 @@
|
|||
import fetch from 'node-fetch';
|
||||
import { exec } from 'child_process';
|
||||
import { promisify } from 'util';
|
||||
import type {
|
||||
|
|
@ -10,6 +9,35 @@ import type {
|
|||
|
||||
const execAsync = promisify(exec);
|
||||
|
||||
/**
|
||||
* Escape a string for use as a single-quoted shell argument.
|
||||
*
|
||||
* In POSIX shells, single-quoted strings treat ALL characters literally
|
||||
* except for the single quote itself. To include a literal single quote,
|
||||
* we use the pattern: end quote + escaped quote + start quote = '\''
|
||||
*
|
||||
* Example: "It's $100" becomes 'It'\''s $100'
|
||||
* Shell interprets: 'It' + \' + 's $100' = It's $100
|
||||
*
|
||||
* This handles ALL shell-special characters including:
|
||||
* - $ (variable expansion)
|
||||
* - ` (command substitution)
|
||||
* - ! (history expansion)
|
||||
* - ? * [ ] (glob patterns)
|
||||
* - ( ) { } (subshell/brace expansion)
|
||||
* - < > | & ; (redirection/control)
|
||||
* - \ " # ~ newlines
|
||||
*
|
||||
* @param arg - The string to escape
|
||||
* @returns The escaped string (without surrounding quotes - caller adds those)
|
||||
*/
|
||||
export function escapeShellArg(arg: string): string {
|
||||
// Replace single quotes with the escape sequence: '\''
|
||||
// This ends the current single-quoted string, adds an escaped literal quote,
|
||||
// and starts a new single-quoted string.
|
||||
return arg.replace(/'/g, "'\\''");
|
||||
}
|
||||
|
||||
export class HindsightClient {
|
||||
private bankId: string = 'default'; // Always use default bank
|
||||
private llmProvider: string;
|
||||
|
|
@ -49,7 +77,7 @@ export class HindsightClient {
|
|||
return;
|
||||
}
|
||||
|
||||
const escapedMission = mission.replace(/'/g, "'\\''"); // Escape single quotes
|
||||
const escapedMission = escapeShellArg(mission);
|
||||
const embedCmd = this.getEmbedCommandPrefix();
|
||||
const cmd = `${embedCmd} --profile openclaw bank mission ${this.bankId} '${escapedMission}'`;
|
||||
|
||||
|
|
@ -63,8 +91,8 @@ export class HindsightClient {
|
|||
}
|
||||
|
||||
async retain(request: RetainRequest): Promise<RetainResponse> {
|
||||
const content = request.content.replace(/'/g, "'\\''"); // Escape single quotes
|
||||
const docId = request.document_id || 'conversation';
|
||||
const content = escapeShellArg(request.content);
|
||||
const docId = escapeShellArg(request.document_id || 'conversation');
|
||||
|
||||
const embedCmd = this.getEmbedCommandPrefix();
|
||||
const cmd = `${embedCmd} --profile openclaw memory retain ${this.bankId} '${content}' --doc-id '${docId}' --async`;
|
||||
|
|
@ -85,7 +113,7 @@ export class HindsightClient {
|
|||
}
|
||||
|
||||
async recall(request: RecallRequest): Promise<RecallResponse> {
|
||||
const query = request.query.replace(/'/g, "'\\''"); // Escape single quotes
|
||||
const query = escapeShellArg(request.query);
|
||||
const maxTokens = request.max_tokens || 1024;
|
||||
|
||||
const embedCmd = this.getEmbedCommandPrefix();
|
||||
|
|
|
|||
Loading…
Reference in a new issue