Skip to content

Commit 818b1dd

Browse files
committed
Harden Skills + CLI SFTP workflow
1 parent 6a631e6 commit 818b1dd

8 files changed

Lines changed: 731 additions & 57 deletions

File tree

electron/bridges/aiBridge.cjs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,13 @@ function buildExternalAgentContextualPrompt({ mode, prompt, chatSessionId, defau
9898
`${scopeHint}` +
9999
`${defaultTargetHint}` +
100100
`Use Skills + CLI instead of the "netcatty-remote-hosts" MCP server for Netcatty session access. ` +
101+
`First classify the task: remote command execution tasks go through \`exec\`, while remote file or directory tasks go through \`sftp\`. If the user explicitly says to avoid shell or \`exec\`, do not use \`exec\`. ` +
101102
`${discoveryHint}` +
102103
`After choosing a target session ID, call \`${cliCommandPrefix} session --session <id> --json${chatSessionId ? ` --chat-session ${chatSessionId}` : ""}\` before executing anything. Do not infer protocol, shell type, device type, or connection readiness from the \`env\` result alone when you are about to run a command. ` +
104+
`For remote file operations, use the Netcatty SFTP CLI surface instead of trying to reconstruct SSH credentials or open your own SSH/SFTP connection. Treat file and directory tasks as SFTP tasks by default, not shell tasks. Prefer one-off commands such as \`${cliCommandPrefix} sftp list --session <id> --remote-path <remote-path> --json${chatSessionId ? ` --chat-session ${chatSessionId}` : ""}\`, \`${cliCommandPrefix} sftp read --session <id> --remote-path <remote-path> --json${chatSessionId ? ` --chat-session ${chatSessionId}` : ""}\`, \`${cliCommandPrefix} sftp write --session <id> --remote-path <remote-path> --content <text> --json${chatSessionId ? ` --chat-session ${chatSessionId}` : ""}\`, \`${cliCommandPrefix} sftp download --session <id> --remote-path <remote-path> --local-path <local-path> --json${chatSessionId ? ` --chat-session ${chatSessionId}` : ""}\`, or \`${cliCommandPrefix} sftp upload --session <id> --local-path <local-path> --remote-path <remote-path> --json${chatSessionId ? ` --chat-session ${chatSessionId}` : ""}\`. ` +
105+
`Keep local and remote path semantics strict: \`--remote-path\` always refers to the remote host, while \`--local-path\` always refers to the local machine running Netcatty. If the user asks to download a file to a local destination such as \`/tmp\`, \`~/Downloads\`, or a desktop path, use \`sftp download\`, not \`sftp read\` or \`sftp write\`. If the user asks to create or modify a file on the remote host, use \`sftp write\` or another remote SFTP operation, not \`sftp download\`. ` +
106+
`If you need to create or update a small text file with known content on the remote host, prefer \`${cliCommandPrefix} sftp write ...\` directly. Use \`sftp upload\` only when a real local file already exists and must be transferred to the remote host. Do not create temporary local files just to upload text that could be sent with \`sftp write\`. ` +
107+
`Keep SFTP usage one-off and explicit: every \`sftp\` command should include both \`--session <id>\` and \`--chat-session ${chatSessionId || "<chat-session-id>"}\`. Do not open reusable SFTP handles or use \`--sftp <id>\`. ` +
103108
`Run Netcatty CLI calls strictly one at a time. Do not issue concurrent or background Netcatty CLI commands for the same chat session, and always wait for each call to finish before starting the next one. ` +
104109
`For simple read-only requests such as hostname, IP address, CPU info, memory info, disk usage, pwd, whoami, uname, or process checks, use the shortest possible path: one \`env\`, one \`session\`, then one \`exec\`. Prefer a single straightforward command over creating helper scripts or multi-step shell orchestration. ` +
105110
`For those simple read-only requests, do not spend time reading extra files, designing scripts, or narrating a plan unless the first direct command fails or the session metadata shows a special device type. ` +

electron/bridges/mcpServerBridge.cjs

Lines changed: 184 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ const { toUnpackedAsarPath } = require("./ai/shellUtils.cjs");
1616
const { execViaPty, execViaChannel, execViaRawPty } = require("./ai/ptyExec.cjs");
1717
const { safeSend } = require("./ipcUtils.cjs");
1818
const { getCliDiscoveryFilePath } = require("../cli/discoveryPath.cjs");
19+
const sftpBridge = require("./sftpBridge.cjs");
1920

2021
const DEBUG_MCP = process.env.NETCATTY_MCP_DEBUG === "1";
2122

@@ -500,6 +501,12 @@ async function handleMessage(socket, line) {
500501
// Methods that modify remote state — blocked in observer mode
501502
const WRITE_METHODS = new Set([
502503
"netcatty/exec",
504+
"netcatty/sftp/write",
505+
"netcatty/sftp/upload",
506+
"netcatty/sftp/mkdir",
507+
"netcatty/sftp/delete",
508+
"netcatty/sftp/rename",
509+
"netcatty/sftp/chmod",
503510
]);
504511

505512
/**
@@ -527,27 +534,55 @@ async function dispatch(method, params) {
527534
return { ok: false, error: "Operation cancelled: the ACP session was stopped." };
528535
}
529536

537+
// Scope validation for session-targeted operations
538+
if (method !== "netcatty/getContext" && params?.sessionId) {
539+
const scopeErr = validateSessionScope(params.sessionId, params?.chatSessionId);
540+
if (scopeErr) return { ok: false, error: scopeErr };
541+
}
542+
530543
// Confirm mode: request user approval for write operations
531544
if (permissionMode === "confirm" && WRITE_METHODS.has(method)) {
545+
if (!params?.chatSessionId) {
546+
return {
547+
ok: false,
548+
error: 'Operation denied: permission mode is "confirm", but this CLI request is not attached to an AI chat session. Manual CLI write operations are non-interactive; switch Settings → AI → Safety to "autonomous" to allow this action.',
549+
};
550+
}
532551
const { chatSessionId, ...toolArgs } = params || {};
533552
const approved = await requestApprovalFromRenderer(method, toolArgs, chatSessionId);
534553
if (!approved) {
535554
return { ok: false, error: "Operation denied by user." };
536555
}
537556
}
538-
539-
// Scope validation for session-targeted operations
540-
if (method !== "netcatty/getContext" && params?.sessionId) {
541-
const scopeErr = validateSessionScope(params.sessionId, params?.chatSessionId);
542-
if (scopeErr) return { ok: false, error: scopeErr };
543-
}
544557
switch (method) {
545558
case "netcatty/getContext":
546559
return handleGetContext(params);
547560
case "netcatty/getStatus":
548561
return handleGetStatus();
549562
case "netcatty/exec":
550563
return handleExec(params);
564+
case "netcatty/sftp/list":
565+
return handleSftpList(params);
566+
case "netcatty/sftp/read":
567+
return handleSftpRead(params);
568+
case "netcatty/sftp/write":
569+
return handleSftpWrite(params);
570+
case "netcatty/sftp/download":
571+
return handleSftpDownload(params);
572+
case "netcatty/sftp/upload":
573+
return handleSftpUpload(params);
574+
case "netcatty/sftp/mkdir":
575+
return handleSftpMkdir(params);
576+
case "netcatty/sftp/delete":
577+
return handleSftpDelete(params);
578+
case "netcatty/sftp/rename":
579+
return handleSftpRename(params);
580+
case "netcatty/sftp/stat":
581+
return handleSftpStat(params);
582+
case "netcatty/sftp/chmod":
583+
return handleSftpChmod(params);
584+
case "netcatty/sftp/home":
585+
return handleSftpHome(params);
551586
case "netcatty/setCancelled":
552587
return handleSetCancelled(params);
553588
default:
@@ -656,6 +691,149 @@ function handleSetCancelled(params) {
656691
};
657692
}
658693

694+
async function withSessionBackedSftp(params, action, options = {}) {
695+
if (!params?.sessionId) throw new Error("sessionId is required");
696+
const opened = await sftpBridge.openSftpForSession(null, { sessionId: params.sessionId });
697+
const sftpId = opened?.sftpId;
698+
if (!sftpId) throw new Error("Failed to open session-backed SFTP handle");
699+
const timeoutMs = Number.isFinite(options.timeoutMs) && options.timeoutMs > 0 ? options.timeoutMs : 0;
700+
const operationName = options.operationName || "SFTP operation";
701+
let timeoutId = null;
702+
let timedOut = false;
703+
try {
704+
const payload = {
705+
...params,
706+
sftpId,
707+
};
708+
if (!timeoutMs) {
709+
return await action(payload);
710+
}
711+
return await new Promise((resolve, reject) => {
712+
let settled = false;
713+
const finishResolve = (value) => {
714+
if (settled) return;
715+
settled = true;
716+
if (timeoutId) clearTimeout(timeoutId);
717+
resolve(value);
718+
};
719+
const finishReject = (err) => {
720+
if (settled) return;
721+
settled = true;
722+
if (timeoutId) clearTimeout(timeoutId);
723+
reject(err);
724+
};
725+
timeoutId = setTimeout(async () => {
726+
if (settled) return;
727+
timedOut = true;
728+
try {
729+
await sftpBridge.closeSftp(null, { sftpId });
730+
} catch {
731+
// Ignore close failures during forced timeout cleanup.
732+
}
733+
finishReject(new Error(`${operationName} timed out after ${timeoutMs}ms`));
734+
}, timeoutMs);
735+
Promise.resolve()
736+
.then(() => action(payload))
737+
.then(finishResolve, finishReject);
738+
});
739+
} finally {
740+
if (timeoutId) clearTimeout(timeoutId);
741+
if (!timedOut) {
742+
try {
743+
await sftpBridge.closeSftp(null, { sftpId });
744+
} catch {
745+
// Ignore close failures for one-off internal SFTP handles.
746+
}
747+
}
748+
}
749+
}
750+
751+
async function handleSftpList(params) {
752+
const entries = await withSessionBackedSftp(params, (payload) => sftpBridge.listSftp(null, payload));
753+
return { ok: true, entries };
754+
}
755+
756+
async function handleSftpRead(params) {
757+
if (!params?.path) throw new Error("path is required");
758+
const content = await withSessionBackedSftp(params, (payload) => sftpBridge.readSftp(null, payload));
759+
return { ok: true, path: params.path, content };
760+
}
761+
762+
async function handleSftpWrite(params) {
763+
if (!params?.path) throw new Error("path is required");
764+
if (typeof params?.content !== "string") throw new Error("content is required");
765+
await withSessionBackedSftp(
766+
params,
767+
(payload) => sftpBridge.writeSftp(null, payload),
768+
{ timeoutMs: commandTimeoutMs, operationName: "SFTP write" },
769+
);
770+
return { ok: true, path: params.path };
771+
}
772+
773+
async function handleSftpDownload(params) {
774+
if (!params?.remotePath || !params?.localPath) {
775+
throw new Error("remotePath and localPath are required");
776+
}
777+
const result = await withSessionBackedSftp(
778+
params,
779+
(payload) => sftpBridge.downloadSftpToLocal(null, payload),
780+
{ timeoutMs: commandTimeoutMs, operationName: "SFTP download" },
781+
);
782+
return { ok: true, ...result };
783+
}
784+
785+
async function handleSftpUpload(params) {
786+
if (!params?.remotePath || !params?.localPath) {
787+
throw new Error("remotePath and localPath are required");
788+
}
789+
const result = await withSessionBackedSftp(
790+
params,
791+
(payload) => sftpBridge.uploadLocalToSftp(null, payload),
792+
{ timeoutMs: commandTimeoutMs, operationName: "SFTP upload" },
793+
);
794+
return { ok: true, ...result };
795+
}
796+
797+
async function handleSftpMkdir(params) {
798+
if (!params?.path) throw new Error("path is required");
799+
await withSessionBackedSftp(params, (payload) => sftpBridge.mkdirSftp(null, payload));
800+
return { ok: true, path: params.path };
801+
}
802+
803+
async function handleSftpDelete(params) {
804+
if (!params?.path) throw new Error("path is required");
805+
await withSessionBackedSftp(params, (payload) => sftpBridge.deleteSftp(null, payload));
806+
return { ok: true, path: params.path };
807+
}
808+
809+
async function handleSftpRename(params) {
810+
if (!params?.oldPath || !params?.newPath) {
811+
throw new Error("oldPath and newPath are required");
812+
}
813+
await withSessionBackedSftp(params, (payload) => sftpBridge.renameSftp(null, payload));
814+
return { ok: true, oldPath: params.oldPath, newPath: params.newPath };
815+
}
816+
817+
async function handleSftpStat(params) {
818+
if (!params?.path) throw new Error("path is required");
819+
const stat = await withSessionBackedSftp(params, (payload) => sftpBridge.statSftp(null, payload));
820+
return { ok: true, stat };
821+
}
822+
823+
async function handleSftpChmod(params) {
824+
if (!params?.path || !params?.mode) throw new Error("path and mode are required");
825+
await withSessionBackedSftp(params, (payload) => sftpBridge.chmodSftp(null, payload));
826+
return { ok: true, path: params.path, mode: params.mode };
827+
}
828+
829+
async function handleSftpHome(params) {
830+
const result = await withSessionBackedSftp(params, (payload) => sftpBridge.getSftpHomeDir(null, payload));
831+
if (!result?.success) {
832+
throw new Error(result?.error || "Could not determine home directory");
833+
}
834+
return { ok: true, homeDir: result.homeDir };
835+
}
836+
659837
// ── Handler: exec ──
660838

661839
function handleExec(params) {

0 commit comments

Comments
 (0)