mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 16:03:26 +00:00
Fix ssh-relay install on hosts with split shell/SFTP namespaces
Strengthen path validation to catch traversal and empty segments in absolute POSIX paths, preventing security issues. Improve split-namespace handling with comprehensive wire tests for uploads and file writes. Ensure system SSH connections bypass namespace mapping entirely rather than attempting incorrect retargeting.
This commit is contained in:
@@ -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',
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -243,14 +243,18 @@ function connectionWithClient(client: Client): SshConnection {
|
||||
return connection
|
||||
}
|
||||
|
||||
async function boundedUpload(operation: Promise<void>, operations: string[]): Promise<void> {
|
||||
async function boundedTransfer(
|
||||
operation: Promise<void>,
|
||||
operations: string[],
|
||||
label: string
|
||||
): Promise<void> {
|
||||
let timeout: ReturnType<typeof setTimeout> | undefined
|
||||
try {
|
||||
await Promise.race([
|
||||
operation,
|
||||
new Promise<never>((_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<void>, 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<void>
|
||||
): Promise<void> {
|
||||
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)
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
Reference in New Issue
Block a user