From f3055c6617e53613df959e9f16d85539b2c38a66 Mon Sep 17 00:00:00 2001 From: AgentRun Artificer Date: Thu, 11 Jun 2026 17:15:58 +0800 Subject: [PATCH] fix: harden ssh download stat diagnostics --- scripts/src/output.ts | 15 +++++++- scripts/src/ssh-file-transfer.ts | 37 ++++++++++++++----- scripts/ssh-argv-guidance-contract-test.ts | 42 ++++++++++++++++++++-- 3 files changed, 82 insertions(+), 12 deletions(-) diff --git a/scripts/src/output.ts b/scripts/src/output.ts index 7433ba1e..6c02c009 100644 --- a/scripts/src/output.ts +++ b/scripts/src/output.ts @@ -67,11 +67,24 @@ function normalizeErrorPayload(command: string, error: unknown): Record | null { + if (error.name !== "SshFileTransferError") return null; + const details = (error as Error & { details?: unknown }).details; + if (typeof details !== "object" || details === null || Array.isArray(details)) return null; + return { details }; +} + function parseStructuredErrorMessage(command: string, message: string): Record | null { if (!shouldSuppressStack(command) && !shouldSuppressStack(commandPrefixFromMessage(message))) return null; const jsonStart = message.indexOf("{"); diff --git a/scripts/src/ssh-file-transfer.ts b/scripts/src/ssh-file-transfer.ts index 4483a0c6..c46c4e15 100644 --- a/scripts/src/ssh-file-transfer.ts +++ b/scripts/src/ssh-file-transfer.ts @@ -354,7 +354,7 @@ async function statRemoteFile( remotePath: string, ): Promise { const stat = await checkedFileTransfer(invocation, executor, builders, "stat", [remotePath]); - return parseFileTransferStat(stat.stdout, remotePath); + return parseFileTransferStat(stat.stdout, stat.stderr, remotePath); } async function checkedFileTransfer( @@ -373,8 +373,8 @@ async function checkedFileTransfer( operation, args: args.slice(0, 4), exitCode: result.exitCode, - stdout: result.stdout.slice(-2000), - stderr: result.stderr.slice(-4000), + stdout: transferTextSnapshot(result.stdout, { headChars: operation === "stat" ? 500 : 120, tailChars: 500 }), + stderr: transferTextSnapshot(result.stderr, { headChars: 120, tailChars: 1000 }), }); } @@ -389,16 +389,35 @@ function buildFileTransferRemoteCommand( return builders.buildRouteCommand(route, ["sh", "-c", posixFileTransferScript(), "unidesk-file-transfer", operation, ...args], { stdin: hasInput }); } -function parseFileTransferStat(stdout: string, remotePath: string): SshFileTransferStat { - const [bytesText, sha256] = stdout.trim().split(/\s+/u); - const bytes = Number(bytesText); - if (!Number.isSafeInteger(bytes) || bytes < 0 || !/^[0-9a-f]{64}$/u.test(sha256 ?? "")) { +function parseFileTransferStat(stdout: string, stderr: string, remotePath: string): SshFileTransferStat { + const lines = stdout.split(/\r?\n/u).map((line) => line.trim()).filter((line) => line.length > 0); + for (const line of lines) { + const [bytesText, sha256, extra] = line.split(/\s+/u); + const bytes = Number(bytesText); + if (extra === undefined && Number.isSafeInteger(bytes) && bytes >= 0 && /^[0-9a-f]{64}$/u.test(sha256 ?? "")) { + return { bytes, sha256: sha256! }; + } + } + { throw new SshFileTransferError("remote file transfer stat returned invalid metadata", { remotePath, - stdout: stdout.slice(0, 500), + expected: " ", + stdout: transferTextSnapshot(stdout, { headChars: 500, tailChars: 500 }), + stderr: transferTextSnapshot(stderr, { headChars: 120, tailChars: 1000 }), + candidateLines: lines.slice(0, 8).map((line) => line.slice(0, 200)), + candidateLineCount: lines.length, }); } - return { bytes, sha256: sha256! }; +} + +function transferTextSnapshot(value: string, limits: { headChars: number; tailChars: number }): Record { + return { + bytes: Buffer.byteLength(value, "utf8"), + chars: value.length, + head: value.slice(0, limits.headChars), + tail: value.slice(Math.max(0, value.length - limits.tailChars)), + truncated: value.length > limits.headChars + limits.tailChars, + }; } function assertTransferStat(label: string, pathName: string, expected: SshFileTransferStat, actual: SshFileTransferStat): void { diff --git a/scripts/ssh-argv-guidance-contract-test.ts b/scripts/ssh-argv-guidance-contract-test.ts index f042486f..e1492888 100644 --- a/scripts/ssh-argv-guidance-contract-test.ts +++ b/scripts/ssh-argv-guidance-contract-test.ts @@ -8,6 +8,7 @@ import { sshHelp } from "./src/help"; import { runApplyPatchV2, type ApplyPatchV2TimingSummary, type ApplyPatchV2BulkReplacementWritePlan } from "./src/apply-patch-v2"; import { providerTriageRecommendedCrossChecks } from "./src/provider-triage"; import { extractRemoteCliOptions, remoteSshFrontendPlanForTest } from "./src/remote"; +import { emitError } from "./src/output"; import { runSshFileTransferOperation, type SshFileTransferCommandBuilders, type SshRemoteCommandExecutor } from "./src/ssh-file-transfer"; import { formatSshFailureHint, @@ -430,7 +431,7 @@ async function applyPatchV2FsBulkFixtureAttempt(patch: string, files: Record = {}, options: { emptyReadOnce?: Record; shortReadOnce?: Record> } = {}): { +function fileTransferFixture(initial: Record = {}, options: { emptyReadOnce?: Record; shortReadOnce?: Record>; statStdoutPrefix?: string; statStdoutSuffix?: string } = {}): { state: Map; commands: Array<{ operation: string; stdin: boolean }>; executor: SshRemoteCommandExecutor; @@ -459,7 +460,7 @@ function fileTransferFixture(initial: Record = {}, options: { em if (operation === "stat") { const content = state.get(target); if (content === undefined) return { exitCode: 1, stdout: "", stderr: "missing" }; - return { exitCode: 0, stdout: `${content.length} ${sha256BufferHex(content)}\n`, stderr: "" }; + return { exitCode: 0, stdout: `${options.statStdoutPrefix ?? ""}${content.length} ${sha256BufferHex(content)}\n${options.statStdoutSuffix ?? ""}`, stderr: "" }; } if (operation === "read-b64-block") { const content = state.get(target); @@ -642,6 +643,43 @@ export async function runSshArgvGuidanceContract(): Promise { assertCondition(readFileSync(localDownload).equals(payload), "download must preserve binary and UTF-8 bytes locally", { commands: transfer.commands }); assertCondition(transfer.commands.some((item) => item.operation === "stat") && transfer.commands.some((item) => item.operation === "read-b64-block"), "file transfer should use stat plus chunked verified reads", transfer.commands); + const noisyStatDownload = path.join(transferRoot, "downloaded", "noisy-stat-copy.bin"); + const noisyStatTransfer = fileTransferFixture({ "/tmp/noisy-stat.bin": payload }, { + statStdoutPrefix: "provider note before stat\n", + statStdoutSuffix: "provider note after stat\n", + }); + const noisyStatResult = await captureStdout(() => runSshFileTransferOperation(parseSshInvocation("D601", ["download", "/tmp/noisy-stat.bin", noisyStatDownload]), ["download", "/tmp/noisy-stat.bin", noisyStatDownload], noisyStatTransfer.executor, noisyStatTransfer.builders)); + const noisyStatJson = JSON.parse(noisyStatResult.stdout) as JsonRecord; + assertCondition(noisyStatResult.exitCode === 0 && noisyStatJson.sha256 === sha256BufferHex(payload), "download stat parser should ignore unrelated stdout lines around the metadata record", noisyStatResult); + + let missingStatError: unknown = null; + const missingStatTransfer = fileTransferFixture(); + const missingStatDownload = path.join(transferRoot, "missing.bin"); + try { + await captureStdout(() => runSshFileTransferOperation(parseSshInvocation("D601", ["download", "/tmp/missing-stat.bin", missingStatDownload]), ["download", "/tmp/missing-stat.bin", missingStatDownload], missingStatTransfer.executor, missingStatTransfer.builders)); + } catch (error) { + missingStatError = error; + } + const missingDetails = (missingStatError as { details?: JsonRecord }).details ?? {}; + const missingStderr = missingDetails.stderr as JsonRecord | undefined; + assertCondition( + missingStatError instanceof Error + && missingStatError.name === "SshFileTransferError" + && missingDetails.operation === "stat" + && missingDetails.exitCode === 1 + && missingStderr?.tail === "missing", + "stat failures should preserve bounded diagnostic details on the thrown file-transfer error", + { name: (missingStatError as Error | null)?.name, details: missingDetails }, + ); + const emittedMissingStat = await captureStdout(async () => { + emitError("ssh D601 download", missingStatError); + return 0; + }); + const emittedMissingJson = JSON.parse(emittedMissingStat.stdout) as JsonRecord; + const emittedError = emittedMissingJson.error as JsonRecord; + const emittedDetails = emittedError.details as JsonRecord; + assertCondition(emittedDetails.operation === "stat" && emittedDetails.exitCode === 1 && (emittedDetails.stderr as JsonRecord).tail === "missing", "CLI JSON error should include bounded file-transfer details by default", emittedMissingJson); + const retryDownload = path.join(transferRoot, "downloaded", "retry-copy.bin"); const retryPayload = Buffer.from("0123456789abcdef".repeat(4096), "utf8"); const retryTransfer = fileTransferFixture({ "/tmp/retry-remote.bin": retryPayload }, { emptyReadOnce: { "/tmp/retry-remote.bin": [1] } });