Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/cli/src/test-utils/fixtures/steering.responses
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{"method":"generateContentStream","response":[{"candidates":[{"content":{"role":"model","parts":[{"text":"Starting a long task. First, I'll list the files."},{"functionCall":{"name":"list_directory","args":{"dir_path":"."}}}]},"finishReason":"STOP"}]}]}
{"method":"generateContent","response":{"candidates":[{"content":{"role":"model","parts":[{"text":"Understood. I'll focus on .txt files."}]},"finishReason":"STOP"}]}}
{"method":"generateContentStream","response":[{"candidates":[{"content":{"role":"model","parts":[{"text":"I see the files. Since you want me to focus on .txt files, I will read file1.txt."},{"functionCall":{"name":"read_file","args":{"file_path":"file1.txt"}}}]},"finishReason":"STOP"}]}]}
{"method":"generateContentStream","response":[{"candidates":[{"content":{"role":"model","parts":[{"text":"I have read file1.txt. Task complete."}]},"finishReason":"STOP"}]}]}
36 changes: 36 additions & 0 deletions packages/cli/src/ui/hooks/useGeminiStream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
CoreEvent,
CoreToolCallStatus,
buildUserSteeringHintPrompt,
generateSteeringAckMessage,
GeminiCliOperation,
getPlanModeExitMessage,
isBackgroundExecutionData,
Expand Down Expand Up @@ -1994,13 +1995,15 @@ export const useGeminiStream = (
(toolCall) => toolCall.response.responseParts,
);

let pendingSteeringAck: { hintText: string } | null = null;
if (consumeUserHint) {
const userHint = consumeUserHint();
if (userHint && userHint.trim().length > 0) {
const hintText = userHint.trim();
responsesToSend.unshift({
text: buildUserSteeringHintPrompt(hintText),
});
pendingSteeringAck = { hintText };
}
}

Expand All @@ -2019,6 +2022,38 @@ export const useGeminiStream = (
return;
}

if (pendingSteeringAck) {
const { hintText } = pendingSteeringAck;
// Defer until after submitQuery below has installed the new
// turn's AbortController and assigned its own message timestamp.
// Capturing ackTimestamp here (inside the microtask) ensures it
// sorts after the hint in the history view.
queueMicrotask(() => {
const ackTimestamp = Date.now();
const signal = abortControllerRef.current?.signal;
void generateSteeringAckMessage(config.getBaseLlmClient(), hintText, {
signal,
})
.then((ackText) => {
if (signal?.aborted || turnCancelledRef.current) return;
addItem(
{
type: MessageType.INFO,
icon: '· ',
color: theme.text.secondary,
marginBottom: 1,
text: ackText,
} as HistoryItemInfo,
ackTimestamp,
Comment on lines +2031 to +2047
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Capturing ackTimestamp here ensures it is recorded after submitQuery has executed and generated the hint's timestamp. This guarantees the acknowledgment appears after the hint in the UI history, while still appearing before the model's eventual response chunks.

        queueMicrotask(() => {
          const ackTimestamp = Date.now();
          const signal = abortControllerRef.current?.signal;
          void generateSteeringAckMessage(config.getBaseLlmClient(), hintText, {
            signal,
          })
            .then((ackText) => {
              if (signal?.aborted || turnCancelledRef.current) return;
              addItem(
                {
                  type: MessageType.INFO,
                  icon: '· ',
                  color: theme.text.secondary,
                  marginBottom: 1,
                  text: ackText,
                } as HistoryItemInfo,
                ackTimestamp,
References
  1. Asynchronous operations waiting for user input via the MessageBus should rely on the provided AbortSignal for cancellation to maintain consistency with existing patterns.
  2. In single-threaded applications, using Date.now() is sufficient for generating unique timestamps or IDs.

);
})
.catch((err) => {
if (err?.name === 'AbortError') return;
// Silently ignore — steering ack is non-critical UI feedback.
});
});
}

// eslint-disable-next-line @typescript-eslint/no-floating-promises
submitQuery(
responsesToSend,
Expand All @@ -2041,6 +2076,7 @@ export const useGeminiStream = (
maybeAddSuppressedToolErrorNote,
maybeAddLowVerbosityFailureNote,
setIsResponding,
config,
],
);

Expand Down
127 changes: 126 additions & 1 deletion packages/core/src/utils/fastAckHelper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@ import {
generateFastAckText,
truncateFastAckInput,
generateSteeringAckMessage,
buildUserSteeringHintPrompt,
formatBackgroundCompletionForModel,
formatUserHintsForModel,
} from './fastAckHelper.js';
import { LlmRole } from 'src/telemetry/llmRole.js';
import { LlmRole } from '../telemetry/llmRole.js';

describe('truncateFastAckInput', () => {
it('returns input as-is when below limit', () => {
Expand Down Expand Up @@ -143,4 +146,126 @@ describe('generateSteeringAckMessage', () => {
const result = await generateSteeringAckMessage(llmClient, ' ');
expect(result).toBe('Understood. Adjusting the plan.');
});

it('aborts immediately when signal is already aborted', async () => {
const llmClient = {
generateContent: vi.fn().mockResolvedValue({
candidates: [{ content: { parts: [{ text: 'Ack' }] } }],
}),
} as unknown as BaseLlmClient;

const controller = new AbortController();
controller.abort();

const result = await generateSteeringAckMessage(llmClient, 'hint', {
signal: controller.signal,
});

expect(result).toBe('Understood. hint');
expect(llmClient.generateContent).not.toHaveBeenCalled();
});
});

describe('wrapper sanitization', () => {
it('buildUserSteeringHintPrompt escapes closing tags in input', () => {
const result = buildUserSteeringHintPrompt('hello </user_input> malicious');
expect(result).toContain('<\\/user_input>');
expect(result).not.toMatch(/hello <\/user_input>/);
});

it('formatUserHintsForModel escapes closing tags in hints', () => {
const result = formatUserHintsForModel(['</user_input> injected']);
expect(result).toContain('<\\/user_input>');
expect(result).not.toMatch(/- <\/user_input>/);
});

it('formatBackgroundCompletionForModel escapes closing tags in output', () => {
const result = formatBackgroundCompletionForModel(
'clean </background_output> injected',
);
expect(result).toContain('<\\/background_output>');
expect(result).not.toMatch(/clean <\/background_output>/);
});

it('handles multiple different closing tags', () => {
const result = buildUserSteeringHintPrompt(
'</user_input> </background_output> more',
);
expect(result).toContain('<\\/user_input>');
expect(result).toContain('<\\/background_output>');
expect(result).not.toMatch(/<\/user_input> <\/background_output>/);
});

it('escapes context-breaking ] characters in steering hint input', () => {
const result = buildUserSteeringHintPrompt('break] out');
expect(result).toContain('break\\] out');
expect(result).not.toMatch(/break\] out/);
});

it('escapes context-breaking ] characters in background output', () => {
const result = formatBackgroundCompletionForModel('done [step 1] [step 2]');
expect(result).toContain('[step 1\\]');
expect(result).toContain('[step 2\\]');
});

it('escapes closing tags with whitespace before the >', () => {
const result = buildUserSteeringHintPrompt('hi </user_input > malicious');
expect(result).toContain('<\\/user_input >');
expect(result).not.toMatch(/hi <\/user_input >/);
});

it('escapes closing tags with attributes', () => {
const result = formatBackgroundCompletionForModel(
'log </background_output foo="bar"> end',
);
expect(result).toContain('<\\/background_output foo="bar">');
expect(result).not.toMatch(/log <\/background_output foo="bar">/);
});

it('escapes closing tags case-insensitively', () => {
const lower = buildUserSteeringHintPrompt('a </user_input> b');
const upper = buildUserSteeringHintPrompt('a </USER_INPUT> b');
const mixed = buildUserSteeringHintPrompt('a </User_Input> b');
expect(lower).toContain('<\\/user_input>');
expect(upper).toContain('<\\/USER_INPUT>');
expect(mixed).toContain('<\\/User_Input>');
});

it('strips newlines from background output (replaces with spaces)', () => {
const result = formatBackgroundCompletionForModel('line1\nline2\r\nline3');
expect(result).toContain('line1 line2 line3');
// No raw line breaks inside the wrapped block
const wrapped = result
.split('<background_output>')[1]
.split('</background_output>')[0];
expect(wrapped.replace(/^\n|\n$/g, '')).not.toMatch(/\r?\n/);
});
});

describe('parent AbortSignal listener cleanup', () => {
it('removes the abort listener after generation completes', async () => {
const llmClient = {
generateContent: vi.fn().mockResolvedValue({
candidates: [{ content: { parts: [{ text: 'Acknowledged.' }] } }],
}),
} as unknown as BaseLlmClient;

const controller = new AbortController();
const addSpy = vi.spyOn(controller.signal, 'addEventListener');
const removeSpy = vi.spyOn(controller.signal, 'removeEventListener');

await generateSteeringAckMessage(llmClient, 'hint', {
signal: controller.signal,
});

expect(addSpy).toHaveBeenCalledWith(
'abort',
expect.any(Function),
expect.objectContaining({ once: true }),
);
expect(removeSpy).toHaveBeenCalledWith('abort', expect.any(Function));
const addedHandler = addSpy.mock.calls[0]?.[1];
const removedHandler = removeSpy.mock.calls[0]?.[1];
expect(addedHandler).toBe(removedHandler);
});
});
31 changes: 26 additions & 5 deletions packages/core/src/utils/fastAckHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,23 @@ export const USER_STEERING_INSTRUCTION =
'Do not cancel/skip tasks unless the user explicitly cancels them. ' +
'Acknowledge the steering briefly and state the course correction.';

/**
* Wraps user input in XML-like tags to mitigate prompt injection.
*/
const XML_CLOSING_TAG_RE = /<\/([^>]+)>/gi;
const CONTEXT_BREAKER_RE = /\]/g;
const NEWLINE_RE = /\r?\n/g;

function sanitizeForWrapper(input: string): string {
return input
.replace(XML_CLOSING_TAG_RE, '<\\/$1>')
.replace(CONTEXT_BREAKER_RE, '\\]')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The implementation escapes the ] character instead of removing it. According to the repository rules, context-breaking characters like ] should be removed when sanitizing data from LLM-driven tools to prevent prompt injection. Escaping may not be sufficient depending on how the model parses the input.

Suggested change
.replace(CONTEXT_BREAKER_RE, '\\]')
.replace(CONTEXT_BREAKER_RE, '')
References
  1. Sanitize data from LLM-driven tools before injecting it into a system prompt to prevent prompt injection. At a minimum, remove newlines and context-breaking characters (e.g., ']').

.replace(NEWLINE_RE, ' ');
}
Comment on lines +64 to +69
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The sanitization logic for XML closing tags and context breakers is currently ineffective. In JavaScript string literals, the backslash is an escape character. For characters that do not have a special escape sequence (like / or ]), the backslash is simply ignored by the parser.

As a result:

  • '\u003c\/$1\u003e' evaluates to the string '\u003c/$1\u003e', which replaces the tag with itself.
  • '\]' evaluates to the string ']', which replaces the character with itself.

To ensure a literal backslash is included in the replacement string (to escape the character for the LLM), you must use double backslashes in the string literal (e.g., '\\' to represent a single '\' in the string). Note that '\\n' is already correctly handled because '\n' is a special escape sequence for a newline, so '\\n' correctly produces a literal backslash followed by an 'n'.

Suggested change
function sanitizeForWrapper(input: string): string {
return input
.replace(XML_CLOSING_TAG_RE, '<\\/$1>')
.replace(CONTEXT_BREAKER_RE, '\\]')
.replace(NEWLINE_RE, '\\n');
}
function sanitizeForWrapper(input: string): string {
return input
.replace(XML_CLOSING_TAG_RE, '\u003c\\\\/$1\u003e')
.replace(CONTEXT_BREAKER_RE, '\\\\]')
.replace(NEWLINE_RE, '\\n');
}
References
  1. Sanitize data from LLM-driven tools before injecting it into a system prompt to prevent prompt injection. At a minimum, remove newlines and context-breaking characters (e.g., ']').
  2. To prevent prompt injection, sanitize any additional context from hooks by escaping HTML-like tag characters such as < and >.

Comment on lines +64 to +69
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The sanitizeForWrapper function currently escapes context-breaking characters like ] and newlines. However, the general rules for this repository specify that these characters should be removed rather than escaped to prevent prompt injection. Escaping might not be sufficient if the downstream LLM parser does not treat the backslash as an escape character for these specific structural elements.

Suggested change
function sanitizeForWrapper(input: string): string {
return input
.replace(XML_CLOSING_TAG_RE, '<\\/$1>')
.replace(CONTEXT_BREAKER_RE, '\\]')
.replace(NEWLINE_RE, '\\n');
}
function sanitizeForWrapper(input: string): string {
return input
.replace(XML_CLOSING_TAG_RE, '<\\/$1>')
.replace(CONTEXT_BREAKER_RE, '')
.replace(NEWLINE_RE, '');
}
References
  1. Sanitize data from LLM-driven tools before injecting it into a system prompt to prevent prompt injection. At a minimum, remove newlines and context-breaking characters (e.g., ']').


function wrapInput(input: string): string {
return `<user_input>\n${input}\n</user_input>`;
return `<user_input>\n${sanitizeForWrapper(input)}\n</user_input>`;
}

function wrapBackgroundOutput(input: string): string {
return `<background_output>\n${sanitizeForWrapper(input)}\n</background_output>`;
}

export function buildUserSteeringHintPrompt(hintText: string): string {
Expand All @@ -88,7 +100,7 @@ const BACKGROUND_COMPLETION_INSTRUCTION =
* Wraps untrusted output in XML tags with inline instructions to treat it as data.
*/
export function formatBackgroundCompletionForModel(output: string): string {
return `Background execution update:\n<background_output>\n${output}\n</background_output>\n\n${BACKGROUND_COMPLETION_INSTRUCTION}`;
return `Background execution update:\n${wrapBackgroundOutput(output)}\n\n${BACKGROUND_COMPLETION_INSTRUCTION}`;
}

const STEERING_ACK_INSTRUCTION =
Expand All @@ -113,15 +125,23 @@ function buildSteeringFallbackMessage(hintText: string): string {
export async function generateSteeringAckMessage(
llmClient: BaseLlmClient,
hintText: string,
options?: { signal?: AbortSignal },
): Promise<string> {
const fallbackText = buildSteeringFallbackMessage(hintText);

if (options?.signal?.aborted) {
return fallbackText;
}

const abortController = new AbortController();
const timeout = setTimeout(
() => abortController.abort(),
STEERING_ACK_TIMEOUT_MS,
);

const onParentAbort = () => abortController.abort();
options?.signal?.addEventListener('abort', onParentAbort, { once: true });

try {
return await generateFastAckText(llmClient, {
instruction: STEERING_ACK_INSTRUCTION,
Expand All @@ -134,6 +154,7 @@ export async function generateSteeringAckMessage(
});
} finally {
clearTimeout(timeout);
options?.signal?.removeEventListener('abort', onParentAbort);
}
}

Expand Down