diff --git a/src/main/ssh/sftp-namespace-resolution.test.ts b/src/main/ssh/sftp-namespace-resolution.test.ts index f41f33fb7fc..f404d27979d 100644 --- a/src/main/ssh/sftp-namespace-resolution.test.ts +++ b/src/main/ssh/sftp-namespace-resolution.test.ts @@ -214,6 +214,24 @@ describe('resolveSftpTransferPath', () => { ['a relative shell path', { shell: `${SHELL_HOME}/${RELAY_DIR}`.slice(1) }, 'SFTP namespace'], ['an absolute home-relative path', { homeRelativePath: `/${RELAY_DIR}` }, 'SFTP namespace'], ['a traversal segment', { homeRelativePath: `${RELAY_DIR}/../escape` }, 'Unsafe remote path'], + [ + 'a traversal segment in the absolute shell path', + { shell: `${SHELL_HOME}/archive/../${RELAY_DIR}` }, + 'Unsafe remote path' + ], + [ + 'an empty component in the absolute shell path', + { shell: `${SHELL_HOME}//${RELAY_DIR}` }, + 'Unsafe remote path' + ], + [ + 'a dot component in the absolute marker path', + { + shellProbePath: `${SHELL_HOME}/./${RELAY_DIR}/${MARKER}`, + homeRelativeProbePath: `${RELAY_DIR}/${MARKER}` + }, + 'Unsafe remote path' + ], ['a line break in the marker path', { shellProbePath: `${SHELL_HOME}/a\nb` }, 'SFTP namespace'], [ 'a mismatched transfer suffix', diff --git a/src/main/ssh/sftp-namespace-resolution.ts b/src/main/ssh/sftp-namespace-resolution.ts index 5e661855752..1020d915127 100644 --- a/src/main/ssh/sftp-namespace-resolution.ts +++ b/src/main/ssh/sftp-namespace-resolution.ts @@ -34,6 +34,13 @@ function assertAbsolutePosixPath(label: string, value: string): void { `SFTP namespace ${label} must be an absolute POSIX path: ${JSON.stringify(redactRelayInstallMarkerTokens(value))}` ) } + // Same segment hygiene as REALPATH start paths — empty/`.`/`..` are programming errors. + if (value === '/') { + return + } + for (const segment of value.slice(1).split('/')) { + assertSafeRemotePathSegment(segment, 'posix') + } } function assertHomeRelativePosixPath(label: string, value: string): void { diff --git a/src/main/ssh/ssh-connection-sftp-wire.test.ts b/src/main/ssh/ssh-connection-sftp-wire.test.ts index 8c945a7f523..9f99dc22233 100644 --- a/src/main/ssh/ssh-connection-sftp-wire.test.ts +++ b/src/main/ssh/ssh-connection-sftp-wire.test.ts @@ -243,14 +243,18 @@ function connectionWithClient(client: Client): SshConnection { return connection } -async function boundedUpload(operation: Promise, operations: string[]): Promise { +async function boundedTransfer( + operation: Promise, + operations: string[], + label: string +): Promise { let timeout: ReturnType | undefined try { await Promise.race([ operation, new Promise((_resolve, reject) => { timeout = setTimeout( - () => reject(new Error(`SFTP wire upload timed out: ${operations.join(', ')}`)), + () => reject(new Error(`SFTP wire ${label} timed out: ${operations.join(', ')}`)), 5_000 ) }) @@ -260,12 +264,23 @@ async function boundedUpload(operation: Promise, operations: string[]): Pr } } -it('uploads through a verified split namespace over a real ssh2 SFTP session', async () => { - const localDir = await realpath(await mkdtemp(join(tmpdir(), 'orca-sftp-wire-local-'))) +function splitNamespaceMapping(homeRelativePath: string): SftpNamespacePathMapping { + return { + homeRelativePath, + shellProbePath: `${SHELL_RELAY_DIR}/${MARKER_PATH}`, + homeRelativeProbePath: `${RELAY_DIR}/${MARKER_PATH}` + } +} + +async function withSplitNamespaceFixture( + run: (args: { + connection: SshConnection + fixture: SftpWireServer + backingRelayDir: string + }) => Promise +): Promise { const backingRoot = await mkdtemp(join(tmpdir(), 'orca-sftp-wire-remote-')) const backingRelayDir = join(backingRoot, ...RELAY_DIR.split('/')) - await mkdir(join(localDir, 'nested')) - await writeFile(join(localDir, 'nested', 'payload.bin'), Buffer.from([0, 1, 2, 255])) await mkdir(join(backingRelayDir, '.install-lock'), { recursive: true }) await writeFile(join(backingRelayDir, '.install-lock', MARKER_FILE), '') @@ -274,34 +289,73 @@ it('uploads through a verified split namespace over a real ssh2 SFTP session', a try { fixture = await startSftpWireServer(backingRoot) client = await connectSshClient(fixture.port) - const connection = connectionWithClient(client) - const mapping: SftpNamespacePathMapping = { - homeRelativePath: RELAY_DIR, - shellProbePath: `${SHELL_RELAY_DIR}/${MARKER_PATH}`, - homeRelativeProbePath: `${RELAY_DIR}/${MARKER_PATH}` - } - - await boundedUpload( - connection.uploadDirectory(localDir, SHELL_RELAY_DIR, { - hostPlatform: getRemoteHostPlatform('linux-x64'), - sftpNamespace: mapping - }), - fixture.operations - ) - - expect(await readFile(join(backingRelayDir, 'nested', 'payload.bin'))).toEqual( - Buffer.from([0, 1, 2, 255]) - ) - expect(fixture.operations).toContain(`REALPATH:.`) - expect(fixture.operations).toContain(`LSTAT:${mapping.shellProbePath}`) - expect(fixture.operations).toContain(`LSTAT:${SFTP_RELAY_DIR}/${MARKER_PATH}`) - expect(fixture.operations).toContain(`MKDIR:${SFTP_RELAY_DIR}/nested`) - expect(fixture.operations).toContain(`OPEN:${SFTP_RELAY_DIR}/nested/payload.bin`) - expect(fixture.operations).toEqual(expect.arrayContaining(['WRITE', 'CLOSE'])) + await run({ + connection: connectionWithClient(client), + fixture, + backingRelayDir + }) } finally { client?.end() await fixture?.close() - await rm(localDir, { recursive: true, force: true }) await rm(backingRoot, { recursive: true, force: true }) } +} + +it('uploads through a verified split namespace over a real ssh2 SFTP session', async () => { + const localDir = await realpath(await mkdtemp(join(tmpdir(), 'orca-sftp-wire-local-'))) + await mkdir(join(localDir, 'nested')) + await writeFile(join(localDir, 'nested', 'payload.bin'), Buffer.from([0, 1, 2, 255])) + + try { + await withSplitNamespaceFixture(async ({ connection, fixture, backingRelayDir }) => { + const mapping = splitNamespaceMapping(RELAY_DIR) + + await boundedTransfer( + connection.uploadDirectory(localDir, SHELL_RELAY_DIR, { + hostPlatform: getRemoteHostPlatform('linux-x64'), + sftpNamespace: mapping + }), + fixture.operations, + 'upload' + ) + + expect(await readFile(join(backingRelayDir, 'nested', 'payload.bin'))).toEqual( + Buffer.from([0, 1, 2, 255]) + ) + expect(fixture.operations).toContain(`REALPATH:.`) + expect(fixture.operations).toContain(`LSTAT:${mapping.shellProbePath}`) + expect(fixture.operations).toContain(`LSTAT:${SFTP_RELAY_DIR}/${MARKER_PATH}`) + expect(fixture.operations).toContain(`MKDIR:${SFTP_RELAY_DIR}/nested`) + expect(fixture.operations).toContain(`OPEN:${SFTP_RELAY_DIR}/nested/payload.bin`) + expect(fixture.operations).toEqual(expect.arrayContaining(['WRITE', 'CLOSE'])) + }) + } finally { + await rm(localDir, { recursive: true, force: true }) + } +}, 15_000) + +it('writes a mapped file through a verified split namespace over a real ssh2 SFTP session', async () => { + await withSplitNamespaceFixture(async ({ connection, fixture, backingRelayDir }) => { + const mapping = splitNamespaceMapping(`${RELAY_DIR}/package.json`) + const shellFilePath = `${SHELL_RELAY_DIR}/package.json` + const contents = '{"name":"orca-relay"}\n' + + await boundedTransfer( + connection.writeFile(shellFilePath, contents, { + hostPlatform: getRemoteHostPlatform('linux-x64'), + sftpNamespace: mapping + }), + fixture.operations, + 'writeFile' + ) + + expect(await readFile(join(backingRelayDir, 'package.json'), 'utf8')).toBe(contents) + expect(fixture.operations).toContain(`REALPATH:.`) + expect(fixture.operations).toContain(`LSTAT:${mapping.shellProbePath}`) + expect(fixture.operations).toContain(`LSTAT:${SFTP_RELAY_DIR}/${MARKER_PATH}`) + expect(fixture.operations).toContain(`OPEN:${SFTP_RELAY_DIR}/package.json`) + expect(fixture.operations).toEqual(expect.arrayContaining(['WRITE', 'CLOSE'])) + // Shell-namespace path must never be opened for a confirmed split write. + expect(fixture.operations).not.toContain(`OPEN:${shellFilePath}`) + }) }, 15_000) diff --git a/src/main/ssh/ssh-relay-sftp-namespace-install.test.ts b/src/main/ssh/ssh-relay-sftp-namespace-install.test.ts index 77c876f59a8..a9e33e62ca1 100644 --- a/src/main/ssh/ssh-relay-sftp-namespace-install.test.ts +++ b/src/main/ssh/ssh-relay-sftp-namespace-install.test.ts @@ -396,14 +396,27 @@ describe('relay install writes on a split SFTP namespace', () => { }) it('leaves system-SSH connections unmapped and unprobed', async () => { - const conn = makeConnection(capture, { systemSsh: true }) + // transferMethods models the shipping SshConnection path (uploadDirectory/writeFile → system SSH helpers). + const conn = makeConnection(capture, { systemSsh: true, transferMethods: true }) feed(POSIX_FIRST_INSTALL) await deployAndLaunchRelay(conn) expect(execCommands().some((command) => MARKER_PATTERN.test(command))).toBe(false) expect(capture.realpathCalls).toEqual([]) + expect(capture.lstatCalls).toEqual([]) + expect(conn.sftp).not.toHaveBeenCalled() + // System SSH never retargets: shell absolute paths, no mapping, no SFTP session. expect(capture.uploadTargets).toEqual([SHELL_RELAY_DIR]) + expect(capture.writePaths).toEqual([ + `${SHELL_RELAY_DIR}/.version`, + `${SHELL_RELAY_DIR}/package.json` + ]) + const transferOptions = [...capture.uploadOptions, ...capture.writeOptions] + expect(transferOptions).toHaveLength(3) + for (const options of transferOptions) { + expect(options?.sftpNamespace).toBeUndefined() + } }) it('leaves Windows hosts unmapped and unprobed', async () => { @@ -521,7 +534,7 @@ describe('relay repair writes on a split SFTP namespace', () => { }) it('does not stamp a marker when repairing over system SSH', async () => { - const conn = makeConnection(capture, { systemSsh: true }) + const conn = makeConnection(capture, { systemSsh: true, transferMethods: true }) feed([ '__ORCA_REMOTE_PLATFORM__ Linux x86_64', SHELL_HOME, @@ -538,7 +551,9 @@ describe('relay repair writes on a split SFTP namespace', () => { await deployAndLaunchRelay(conn) expect(execCommands().some((command) => MARKER_PATTERN.test(command))).toBe(false) + expect(conn.sftp).not.toHaveBeenCalled() expect(capture.writePaths).toEqual([`${SHELL_RELAY_DIR}/package.json`]) + expect(capture.writeOptions).toEqual([expect.objectContaining({ sftpNamespace: undefined })]) }) it('degrades to shell paths when marker creation fails outright', async () => {