fix: harden remote questions — validate IDs before test-send, remove dead code
- Validate channel IDs via isValidChannelId() before URL interpolation in setup wizard, preventing SSRF during test-send - Add 15s fetch timeout to setup API calls (fetchJson, Discord test-send) - Sanitize Discord error responses before surfacing to user - Remove dead send.ts + channels.ts (unused parallel implementation) - Add poll retry tolerance in manager.ts (1 transient error before fail) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
28a30c2cc3
commit
f4c46516a6
4 changed files with 14 additions and 254 deletions
|
|
@ -1,36 +0,0 @@
|
|||
/**
|
||||
* Remote Questions — Adapter pattern interfaces
|
||||
*
|
||||
* Defines the contract for Slack/Discord (or any future) channel adapters.
|
||||
*/
|
||||
|
||||
export interface ChannelAdapter {
|
||||
readonly name: string;
|
||||
sendQuestions(questions: FormattedQuestion[]): Promise<SendResult>;
|
||||
pollResponse(ref: PollReference): Promise<RemoteAnswer | null>;
|
||||
validate(): Promise<void>;
|
||||
}
|
||||
|
||||
export interface FormattedQuestion {
|
||||
id: string;
|
||||
header: string;
|
||||
question: string;
|
||||
options: Array<{ label: string; description: string }>;
|
||||
allowMultiple: boolean;
|
||||
}
|
||||
|
||||
export interface SendResult {
|
||||
ref: PollReference;
|
||||
threadUrl?: string;
|
||||
}
|
||||
|
||||
export interface PollReference {
|
||||
channelType: "slack" | "discord";
|
||||
messageId: string;
|
||||
threadTs?: string;
|
||||
channelId: string;
|
||||
}
|
||||
|
||||
export interface RemoteAnswer {
|
||||
answers: Record<string, { answers: string[]; user_note?: string }>;
|
||||
}
|
||||
|
|
@ -122,14 +122,19 @@ async function pollUntilDone(
|
|||
ref: import("./types.js").RemotePromptRef,
|
||||
signal?: AbortSignal,
|
||||
): Promise<RemoteAnswer | null> {
|
||||
let retryCount = 0;
|
||||
while (Date.now() < prompt.timeoutAt && !signal?.aborted) {
|
||||
try {
|
||||
const answer = await adapter.pollAnswer(prompt, ref);
|
||||
updatePromptRecord(prompt.id, { lastPollAt: Date.now() });
|
||||
retryCount = 0;
|
||||
if (answer) return answer;
|
||||
} catch (err) {
|
||||
markPromptStatus(prompt.id, "failed", sanitizeError(String((err as Error).message)));
|
||||
return null;
|
||||
retryCount++;
|
||||
if (retryCount > 1) {
|
||||
markPromptStatus(prompt.id, "failed", sanitizeError(String((err as Error).message)));
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
await sleep(prompt.pollIntervalMs, signal);
|
||||
|
|
|
|||
|
|
@ -8,7 +8,8 @@ import { CURSOR_MARKER, Editor, type EditorTheme, Key, matchesKey, truncateToWid
|
|||
import { existsSync, readFileSync, writeFileSync, mkdirSync } from "node:fs";
|
||||
import { dirname, join } from "node:path";
|
||||
import { getGlobalGSDPreferencesPath, loadEffectiveGSDPreferences } from "../gsd/preferences.js";
|
||||
import { getRemoteConfigStatus, resolveRemoteConfig } from "./config.js";
|
||||
import { getRemoteConfigStatus, isValidChannelId, resolveRemoteConfig } from "./config.js";
|
||||
import { sanitizeError } from "./manager.js";
|
||||
import { getLatestPromptSummary } from "./status.js";
|
||||
|
||||
export async function handleRemote(
|
||||
|
|
@ -37,6 +38,7 @@ async function handleSetupSlack(ctx: ExtensionCommandContext): Promise<void> {
|
|||
|
||||
const channelId = await promptInput(ctx, "Channel ID", "Paste the Slack channel ID (e.g. C0123456789)");
|
||||
if (!channelId) return void ctx.ui.notify("Slack setup cancelled.", "info");
|
||||
if (!isValidChannelId("slack", channelId)) return void ctx.ui.notify("Invalid Slack channel ID format — expected 9-12 uppercase alphanumeric characters.", "error");
|
||||
|
||||
const send = await fetchJson("https://slack.com/api/chat.postMessage", {
|
||||
method: "POST",
|
||||
|
|
@ -61,15 +63,17 @@ async function handleSetupDiscord(ctx: ExtensionCommandContext): Promise<void> {
|
|||
|
||||
const channelId = await promptInput(ctx, "Channel ID", "Paste the Discord channel ID (e.g. 1234567890123456789)");
|
||||
if (!channelId) return void ctx.ui.notify("Discord setup cancelled.", "info");
|
||||
if (!isValidChannelId("discord", channelId)) return void ctx.ui.notify("Invalid Discord channel ID format — expected 17-20 digit numeric ID.", "error");
|
||||
|
||||
const sendResponse = await fetch(`https://discord.com/api/v10/channels/${channelId}/messages`, {
|
||||
method: "POST",
|
||||
headers: { Authorization: `Bot ${token}`, "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ content: "GSD remote questions connected." }),
|
||||
signal: AbortSignal.timeout(15_000),
|
||||
});
|
||||
if (!sendResponse.ok) {
|
||||
const body = await sendResponse.text().catch(() => "");
|
||||
return void ctx.ui.notify(`Could not send to channel (HTTP ${sendResponse.status}): ${body}`, "error");
|
||||
return void ctx.ui.notify(`Could not send to channel (HTTP ${sendResponse.status}): ${sanitizeError(body).slice(0, 200)}`, "error");
|
||||
}
|
||||
|
||||
saveProviderToken("discord_bot", token);
|
||||
|
|
@ -138,7 +142,7 @@ async function handleRemoteMenu(ctx: ExtensionCommandContext): Promise<void> {
|
|||
|
||||
async function fetchJson(url: string, init?: RequestInit): Promise<any> {
|
||||
try {
|
||||
const response = await fetch(url, init);
|
||||
const response = await fetch(url, { ...init, signal: AbortSignal.timeout(15_000) });
|
||||
return await response.json();
|
||||
} catch {
|
||||
return null;
|
||||
|
|
|
|||
|
|
@ -1,213 +0,0 @@
|
|||
/**
|
||||
* Remote Questions — Entry point
|
||||
*
|
||||
* Transparent routing: when ctx.hasUI is false and a remote channel is
|
||||
* configured, sends questions via Slack/Discord and polls for the response.
|
||||
*
|
||||
* The LLM keeps calling `ask_user_questions` as normal — this module
|
||||
* intercepts the non-interactive branch.
|
||||
*/
|
||||
|
||||
import type { FormattedQuestion, ChannelAdapter, RemoteAnswer } from "./channels.js";
|
||||
import { resolveRemoteConfig, type ResolvedConfig } from "./config.js";
|
||||
import { SlackAdapter } from "./slack-adapter.js";
|
||||
import { DiscordAdapter } from "./discord-adapter.js";
|
||||
|
||||
// ─── Types ───────────────────────────────────────────────────────────────────
|
||||
|
||||
interface Question {
|
||||
id: string;
|
||||
header: string;
|
||||
question: string;
|
||||
options: Array<{ label: string; description: string }>;
|
||||
allowMultiple?: boolean;
|
||||
}
|
||||
|
||||
interface ToolResult {
|
||||
content: Array<{ type: "text"; text: string }>;
|
||||
details?: Record<string, unknown>;
|
||||
}
|
||||
|
||||
// ─── Public API ──────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Try to send questions via a remote channel (Slack/Discord).
|
||||
* Returns a formatted ToolResult if successful, or null if no remote
|
||||
* channel is configured (caller falls back to the original error).
|
||||
*/
|
||||
export async function tryRemoteQuestions(
|
||||
questions: Question[],
|
||||
signal?: AbortSignal,
|
||||
): Promise<ToolResult | null> {
|
||||
const config = resolveRemoteConfig();
|
||||
if (!config) return null;
|
||||
|
||||
const adapter = createAdapter(config);
|
||||
const formatted = questionsToFormatted(questions);
|
||||
|
||||
try {
|
||||
await adapter.validate();
|
||||
} catch (err) {
|
||||
return errorToolResult(`Remote auth failed (${config.channel}): ${(err as Error).message}`);
|
||||
}
|
||||
|
||||
let sendResult;
|
||||
try {
|
||||
sendResult = await adapter.sendQuestions(formatted);
|
||||
} catch (err) {
|
||||
return errorToolResult(`Failed to send questions via ${config.channel}: ${(err as Error).message}`);
|
||||
}
|
||||
|
||||
const threadInfo = sendResult.threadUrl
|
||||
? ` Thread: ${sendResult.threadUrl}`
|
||||
: "";
|
||||
|
||||
// Poll for response
|
||||
const answer = await pollWithTimeout(adapter, sendResult.ref, formatted, signal, config);
|
||||
|
||||
if (!answer) {
|
||||
// Timeout — return structured result so the LLM knows
|
||||
return {
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: JSON.stringify({
|
||||
timed_out: true,
|
||||
channel: config.channel,
|
||||
timeout_minutes: config.timeoutMs / 60000,
|
||||
thread_url: sendResult.threadUrl ?? null,
|
||||
message: `User did not respond within ${config.timeoutMs / 60000} minutes.${threadInfo}`,
|
||||
}),
|
||||
},
|
||||
],
|
||||
details: {
|
||||
remote: true,
|
||||
channel: config.channel,
|
||||
timed_out: true,
|
||||
threadUrl: sendResult.threadUrl,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
// Format the answer in the same structure as formatForLLM
|
||||
const formattedAnswer = formatRemoteAnswerForLLM(answer);
|
||||
|
||||
return {
|
||||
content: [{ type: "text", text: formattedAnswer }],
|
||||
details: {
|
||||
remote: true,
|
||||
channel: config.channel,
|
||||
timed_out: false,
|
||||
threadUrl: sendResult.threadUrl,
|
||||
questions,
|
||||
response: answer,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
// ─── Internal ────────────────────────────────────────────────────────────────
|
||||
|
||||
function createAdapter(config: ResolvedConfig): ChannelAdapter & {
|
||||
pollResponseWithQuestions?: (
|
||||
ref: import("./channels.js").PollReference,
|
||||
questions: FormattedQuestion[],
|
||||
) => Promise<RemoteAnswer | null>;
|
||||
} {
|
||||
switch (config.channel) {
|
||||
case "slack":
|
||||
return new SlackAdapter(config.token, config.channelId);
|
||||
case "discord":
|
||||
return new DiscordAdapter(config.token, config.channelId);
|
||||
default:
|
||||
throw new Error(`Unknown channel type: ${config.channel}`);
|
||||
}
|
||||
}
|
||||
|
||||
async function pollWithTimeout(
|
||||
adapter: ReturnType<typeof createAdapter>,
|
||||
ref: import("./channels.js").PollReference,
|
||||
questions: FormattedQuestion[],
|
||||
signal: AbortSignal | undefined,
|
||||
config: ResolvedConfig,
|
||||
): Promise<RemoteAnswer | null> {
|
||||
const deadline = Date.now() + config.timeoutMs;
|
||||
let retries = 0;
|
||||
const maxNetworkRetries = 1;
|
||||
|
||||
while (Date.now() < deadline && !signal?.aborted) {
|
||||
try {
|
||||
// Use the question-aware poll if available
|
||||
const answer = adapter.pollResponseWithQuestions
|
||||
? await adapter.pollResponseWithQuestions(ref, questions)
|
||||
: await adapter.pollResponse(ref);
|
||||
|
||||
if (answer) return answer;
|
||||
retries = 0; // Reset on successful poll
|
||||
} catch {
|
||||
retries++;
|
||||
if (retries > maxNetworkRetries) return null;
|
||||
}
|
||||
|
||||
await sleep(config.pollIntervalMs, signal);
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
function sleep(ms: number, signal?: AbortSignal): Promise<void> {
|
||||
return new Promise((resolve) => {
|
||||
if (signal?.aborted) {
|
||||
resolve();
|
||||
return;
|
||||
}
|
||||
|
||||
let settled = false;
|
||||
const settle = () => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
clearTimeout(timer);
|
||||
if (signal) signal.removeEventListener("abort", onAbort);
|
||||
resolve();
|
||||
};
|
||||
|
||||
const onAbort = () => settle();
|
||||
const timer = setTimeout(() => settle(), ms);
|
||||
|
||||
if (signal) {
|
||||
signal.addEventListener("abort", onAbort, { once: true });
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
function questionsToFormatted(questions: Question[]): FormattedQuestion[] {
|
||||
return questions.map((q) => ({
|
||||
id: q.id,
|
||||
header: q.header,
|
||||
question: q.question,
|
||||
options: q.options,
|
||||
allowMultiple: q.allowMultiple ?? false,
|
||||
}));
|
||||
}
|
||||
|
||||
/**
|
||||
* Format RemoteAnswer into the same JSON structure as the local formatForLLM.
|
||||
* Structure: { answers: { [id]: { answers: string[] } } }
|
||||
*/
|
||||
function formatRemoteAnswerForLLM(answer: RemoteAnswer): string {
|
||||
const formatted: Record<string, { answers: string[] }> = {};
|
||||
for (const [id, data] of Object.entries(answer.answers)) {
|
||||
const list = [...data.answers];
|
||||
if (data.user_note) {
|
||||
list.push(`user_note: ${data.user_note}`);
|
||||
}
|
||||
formatted[id] = { answers: list };
|
||||
}
|
||||
return JSON.stringify({ answers: formatted });
|
||||
}
|
||||
|
||||
function errorToolResult(message: string): ToolResult {
|
||||
return {
|
||||
content: [{ type: "text", text: message }],
|
||||
details: { remote: true, error: true },
|
||||
};
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue