Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 82 additions & 0 deletions apps/desktop/electron/hardening.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,15 @@ import {
dataUrlReadMaxBytesFromMb,
DEFAULT_FETCH_TIMEOUT_MS,
encryptDesktopSecret,
filenameFromContentDisposition,
PLUGIN_DOWNLOAD_MAX_BYTES,
readFileDataUrlForIpc,
resolveDirectoryForIpc,
resolveDownloadCollision,
resolveReadableFileForIpc,
resolveRequestedPathForIpc,
resolveTimeoutMs,
safeDownloadFilename,
sensitiveFileBlockReason
} from './hardening'

Expand Down Expand Up @@ -352,3 +356,81 @@ test('resolveDirectoryForIpc accepts directory symlinks or junctions', async ()
fs.rmSync(tempDir, { recursive: true, force: true })
}
})

test('safeDownloadFilename strips every path escape a server or DB row could inject', () => {
// Separators become underscores rather than being dropped, so the name stays
// recognisable and can never address a parent directory. The leading-dot
// strip then runs on the result, so a leading '..' loses its dots too.
assert.equal(safeDownloadFilename('../../etc/passwd'), '_.._etc_passwd')
assert.equal(safeDownloadFilename('..\\..\\Windows\\System32\\cfg'), '_.._Windows_System32_cfg')
assert.equal(safeDownloadFilename('/absolute/path.txt'), '_absolute_path.txt')

// Bare traversal tokens and dotfile-forcing prefixes carry no usable name.
assert.equal(safeDownloadFilename('..'), 'download')
assert.equal(safeDownloadFilename('.'), 'download')
assert.equal(safeDownloadFilename('...'), 'download')
assert.equal(safeDownloadFilename('.hidden'), 'hidden')

// Empty / missing / NUL-poisoned input falls back instead of throwing.
assert.equal(safeDownloadFilename(''), 'download')
assert.equal(safeDownloadFilename(null), 'download')
assert.equal(safeDownloadFilename('a\0b.txt'), 'ab.txt')
assert.equal(safeDownloadFilename('', 'fallback.bin'), 'fallback.bin')

// An ordinary name survives untouched.
assert.equal(safeDownloadFilename('report 2026.pdf'), 'report 2026.pdf')
})

test('filenameFromContentDisposition prefers RFC 5987 and sanitizes both forms', () => {
assert.equal(filenameFromContentDisposition('attachment; filename="notes.txt"'), 'notes.txt')
assert.equal(filenameFromContentDisposition('attachment; filename=notes.txt'), 'notes.txt')

// filename* wins when both are present, and percent-decoding is applied.
assert.equal(
filenameFromContentDisposition("attachment; filename=\"fallback.txt\"; filename*=UTF-8''r%C3%A9sum%C3%A9.pdf"),
'résumé.pdf'
)

// A traversal smuggled through either form is still neutralized.
assert.equal(filenameFromContentDisposition('attachment; filename="../../evil.sh"'), '_.._evil.sh')
assert.equal(filenameFromContentDisposition("attachment; filename*=UTF-8''%2e%2e%2f%2e%2e%2fevil.sh"), '_.._evil.sh')

// Malformed percent-encoding falls back to the plain form rather than throwing.
assert.equal(filenameFromContentDisposition("attachment; filename=\"ok.txt\"; filename*=UTF-8''%E0%A4%A"), 'ok.txt')

// No header, or no filename in it, yields empty so the caller can fall back.
assert.equal(filenameFromContentDisposition('attachment'), '')
assert.equal(filenameFromContentDisposition(''), '')
assert.equal(filenameFromContentDisposition(null), '')
})

test('resolveDownloadCollision suffixes before the extension and never clobbers', () => {
const dir = path.join(os.tmpdir(), 'dl')
const taken = new Set([path.join(dir, 'a.txt'), path.join(dir, 'a (1).txt')])
const exists = (candidate: string) => taken.has(candidate)

// A free name is returned untouched.
assert.equal(resolveDownloadCollision(path.join(dir, 'free.txt'), exists), path.join(dir, 'free.txt'))

// Occupied names walk forward past every taken index.
assert.equal(resolveDownloadCollision(path.join(dir, 'a.txt'), exists), path.join(dir, 'a (2).txt'))

// The suffix goes before the extension so the file still opens correctly,
// including for multi-dot names.
const archive = path.join(dir, 'bundle.tar.gz')
assert.equal(resolveDownloadCollision(archive, c => c === archive), path.join(dir, 'bundle.tar (1).gz'))

// A leading-dot file is all name, no extension.
const dotfile = path.join(dir, '.gitignore')
assert.equal(resolveDownloadCollision(dotfile, c => c === dotfile), path.join(dir, '.gitignore (1)'))

// An exhausted range raises instead of spinning forever.
assert.throws(() => resolveDownloadCollision(path.join(dir, 'x.txt'), () => true, 3), /after 3 attempts/)
})

test('the plugin download cap matches the backend attachment limit', () => {
// A blob the backend refused to accept can't come back down, so the ceiling
// tracks KANBAN_ATTACHMENT_MAX_BYTES rather than drifting on its own.
assert.equal(PLUGIN_DOWNLOAD_MAX_BYTES, 25 * 1024 * 1024)
assert.ok(PLUGIN_DOWNLOAD_MAX_BYTES < ATTACHMENT_UPLOAD_DEFAULT_MAX_BYTES)
})
72 changes: 72 additions & 0 deletions apps/desktop/electron/hardening.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ const DATA_URL_READ_MAX_MAX_MB = 4096
// reader so the payload still fits uvicorn's raised ws_max_size (384 MiB)
// after base64 + framing. Preview stays on the Settings-configurable path.
const ATTACHMENT_UPLOAD_DEFAULT_MAX_BYTES = 256 * 1024 * 1024
// Ceiling for a plugin-initiated download buffered in main before it's written
// to disk. Matches the kanban attachment upload cap (KANBAN_ATTACHMENT_MAX_BYTES
// in hermes_cli/kanban_db.py) — a blob the backend refused to accept can't come
// back down — with headroom for other plugins' smaller payloads.
const PLUGIN_DOWNLOAD_MAX_BYTES = 25 * 1024 * 1024
const TEXT_PREVIEW_SOURCE_MAX_BYTES = 64 * 1024 * 1024

function clampDataUrlReadMaxMb(value) {
Expand Down Expand Up @@ -48,6 +53,69 @@ function resolveTimeoutMs(timeoutMs, fallbackMs = DEFAULT_FETCH_TIMEOUT_MS) {
return fallback
}

// A downloaded filename is attacker-influenced data (it comes from a DB row a
// worker wrote, or a Content-Disposition header). It must never be able to
// escape the directory the user picked, so reduce it to a bare basename with
// no separators, no traversal, and no leading dot.
function safeDownloadFilename(name, fallback = 'download') {
const raw = String(name || '')
.replace(/[/\\]/g, '_')
.replace(/\0/g, '')
.trim()

// '.' and '..' survive the separator strip; treat them as absent.
const cleaned = raw === '.' || raw === '..' ? '' : raw.replace(/^\.+/, '')

return cleaned || fallback
}

// RFC 6266 Content-Disposition filename, preferring the RFC 5987 `filename*`
// form when present (it carries the encoding and survives non-ASCII names).
function filenameFromContentDisposition(header) {
const value = String(header || '')

const extended = /filename\*\s*=\s*(?:UTF-8|utf-8)''([^;]+)/.exec(value)

if (extended) {
try {
return safeDownloadFilename(decodeURIComponent(extended[1]), '')
} catch {
// Malformed percent-encoding — fall through to the plain form.
}
}

const plain = /filename\s*=\s*(?:"([^"]*)"|([^;]+))/.exec(value)

return plain ? safeDownloadFilename((plain[1] ?? plain[2] ?? '').trim(), '') : ''
}

// Pick a path that doesn't clobber an existing file: `notes.txt` becomes
// `notes (1).txt`, then `notes (2).txt`. The suffix goes before the extension
// so the file still opens in the right app. `exists` is injected so this stays
// pure and testable; bounded so a pathological directory can't spin forever.
function resolveDownloadCollision(targetPath, exists, limit = 1000) {
if (!exists(targetPath)) {
return targetPath
}

const dir = path.dirname(targetPath)
const base = path.basename(targetPath)
// Leading-dot files ('.gitignore') are all name, no extension.
const dot = base.lastIndexOf('.')
const stem = dot > 0 ? base.slice(0, dot) : base
const ext = dot > 0 ? base.slice(dot) : ''

for (let n = 1; n <= limit; n += 1) {
const candidate = path.join(dir, `${stem} (${n})${ext}`)

if (!exists(candidate)) {
return candidate
}
}

throw new Error(`Could not find a free filename for ${base} after ${limit} attempts.`)
}

function encryptDesktopSecret(value, safeStorageApi) {
const raw = String(value || '')

Expand Down Expand Up @@ -355,12 +423,16 @@ export {
dataUrlReadMaxBytesFromMb,
DEFAULT_FETCH_TIMEOUT_MS,
encryptDesktopSecret,
filenameFromContentDisposition,
PLUGIN_DOWNLOAD_MAX_BYTES,
readFileDataUrlForIpc,
rejectUnsafePathSyntax,
resolveDirectoryForIpc,
resolveDownloadCollision,
resolveReadableFileForIpc,
resolveRequestedPathForIpc,
resolveTimeoutMs,
safeDownloadFilename,
sensitiveFileBlockReason,
TEXT_PREVIEW_SOURCE_MAX_BYTES
}
167 changes: 167 additions & 0 deletions apps/desktop/electron/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,14 @@ import {
dataUrlReadMaxBytesFromMb,
DEFAULT_FETCH_TIMEOUT_MS,
encryptDesktopSecret as encryptDesktopSecretStrict,
filenameFromContentDisposition,
PLUGIN_DOWNLOAD_MAX_BYTES,
readFileDataUrlForIpc,
resolveDownloadCollision,
resolveReadableFileForIpc,
resolveRequestedPathForIpc,
resolveTimeoutMs,
safeDownloadFilename,
TEXT_PREVIEW_SOURCE_MAX_BYTES
} from './hardening'
import { createLinkTitleWindow, guardLinkTitleSession, readLinkTitleWindowTitle } from './link-title-window'
Expand Down Expand Up @@ -4235,6 +4239,82 @@ function fetchJson(url, token, options: any = {}) {
})
}

// Binary sibling of `fetchJson`: same auth header and protocol guard, but the
// response stays a Buffer instead of being decoded as UTF-8 and JSON.parsed.
// Attachments are arbitrary bytes (PNG, PDF, tarball) — routing them through
// the JSON path corrupts them. Capped so a huge blob can't exhaust main's heap.
function fetchBuffer(url, token, options: any = {}) {
return new Promise<{ buffer: Buffer; contentDisposition: string; contentType: string }>((resolve, reject) => {
const parsed = new URL(url)

if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') {
reject(new Error(`Unsupported Hermes backend URL protocol: ${parsed.protocol}`))

return
}

const client = parsed.protocol === 'https:' ? https : http
const timeoutMs = resolveTimeoutMs(options.timeoutMs, DEFAULT_FETCH_TIMEOUT_MS)

const maxBytes =
Number.isFinite(options.maxBytes) && Number(options.maxBytes) > 0
? Number(options.maxBytes)
: PLUGIN_DOWNLOAD_MAX_BYTES

const req = client.request(
parsed,
{
method: 'GET',
headers: {
...(token ? { 'X-Hermes-Session-Token': token } : {}),
...(options.bearer ? { Authorization: `Bearer ${options.bearer}` } : {})
}
},
res => {
const chunks = []
let total = 0

res.on('error', reject)
res.on('data', chunk => {
total += chunk.length

// Abort mid-stream rather than buffering a blob we've already
// decided to refuse.
if (total > maxBytes) {
req.destroy(new Error(`Download exceeds the ${Math.floor(maxBytes / (1024 * 1024))} MB limit.`))

return
}

chunks.push(chunk)
})
res.on('end', () => {
const buffer = Buffer.concat(chunks)

if ((res.statusCode || 500) >= 400) {
// Errors from the backend are JSON/text even on this path.
reject(new Error(`${res.statusCode}: ${buffer.toString('utf8').slice(0, 200) || res.statusMessage}`))

return
}

resolve({
buffer,
contentDisposition: String(res.headers['content-disposition'] || ''),
contentType: String(res.headers['content-type'] || '')
})
})
}
)

req.on('error', reject)
req.setTimeout(timeoutMs, () => {
req.destroy(new Error(`Timed out connecting to Hermes backend after ${timeoutMs}ms`))
})
req.end()
})
}

function fetchPublicJson(url, options: any = {}) {
// Credential-free JSON GET/POST for public gateway endpoints
// (``/api/status``, ``/api/auth/providers``). Unlike ``fetchJson`` it sends
Expand Down Expand Up @@ -10129,6 +10209,93 @@ ipcMain.handle('hermes:api', async (_event, request) => {
})
})

// Plugin binary download: fetch bytes from a plugin's own API namespace, ask
// the user where to put them, write, and offer to reveal. The renderer runs on
// a file:// origin and `hermes:api` is JSON-only, so a plugin has no other way
// to hand the user a file. Namespace scoping is enforced renderer-side by
// `pluginDownload` and re-derived here — main never takes a caller-supplied
// absolute URL.
ipcMain.handle('hermes:plugin:download', async (_event, request) => {
const pluginId = String(request?.pluginId || '')
const suffix = String(request?.path || '')

// Belt-and-braces against a compromised renderer: rebuild the path from the
// plugin id here instead of trusting a caller-assembled one.
if (!/^[a-z0-9][a-z0-9-]*$/i.test(pluginId)) {
throw new Error(`hermes:plugin:download: invalid plugin id "${pluginId}"`)
}

if (!suffix.startsWith('/') || suffix.split(/[?#]/, 1)[0].split('/').includes('..')) {
throw new Error(`hermes:plugin:download: illegal path "${suffix}"`)
}

const profile = request?.profile
const connection = await ensureBackend(resolveRouteProfile(null, profile))

const requestPath = pathWithGlobalRemoteProfile(
`/api/plugins/${pluginId}${suffix}`,
profile,
profileRouteOptions(profile)
)

const url = `${connection.baseUrl}${requestPath}`

let fetched

if (connection.authMode === 'oauth') {
// Cookie-partition downloads would need the electron.net path; the native
// bearer covers the flows we support today. Fail loudly instead of writing
// a 401 body to disk as if it were the file.
const nativeAt = await ensureNativeAccessToken(connection.baseUrl).catch(() => null)

if (!nativeAt) {
throw new Error('Downloads are not supported against cookie-authenticated OAuth backends yet.')
}

fetched = await fetchBuffer(url, null, { bearer: nativeAt, timeoutMs: request?.timeoutMs })
} else {
fetched = await fetchBuffer(url, connection.token, { timeoutMs: request?.timeoutMs })
}

// Caller's suggestion first (the DB's filename), then the server's
// Content-Disposition, then a generic fallback. Every branch is sanitized:
// all three sources are attacker-influenced.
const suggested =
safeDownloadFilename(request?.filename, '') ||
filenameFromContentDisposition(fetched.contentDisposition) ||
'download'

const target = await dialog.showSaveDialog(mainWindow, {
title: 'Save Attachment',
defaultPath: path.join(app.getPath('downloads'), suggested)
})

if (target.canceled || !target.filePath) {
return { canceled: true }
}

// The dialog already warns on overwrite, but a user can still land on a name
// that raced into existence; never clobber silently.
const filePath = resolveDownloadCollision(target.filePath, candidate => fs.existsSync(candidate))
await fs.promises.writeFile(filePath, fetched.buffer)

return { canceled: false, filePath }
})

// Reveal a saved download in Finder/Explorer/Files — the follow-up affordance
// for the toast the renderer shows after a successful save.
ipcMain.handle('hermes:plugin:revealDownload', (_event, filePath) => {
const target = String(filePath || '')

if (!target) {
return false
}

shell.showItemInFolder(target)

return true
})

// One deduper per cross-window cue — the choke point every window shares. Main
// handles IPC serially, so the first window to claim a key wins with no race.
const isDuplicateNotification = createEventDeduper()
Expand Down
2 changes: 2 additions & 0 deletions apps/desktop/electron/preload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ contextBridge.exposeInMainWorld('hermesDesktop', {
set: name => ipcRenderer.invoke('hermes:profile:set', name)
},
api: request => ipcRenderer.invoke('hermes:api', request),
pluginDownload: request => ipcRenderer.invoke('hermes:plugin:download', request),
pluginRevealDownload: filePath => ipcRenderer.invoke('hermes:plugin:revealDownload', filePath),
notify: payload => ipcRenderer.invoke('hermes:notify', payload),
requestMicrophoneAccess: () => ipcRenderer.invoke('hermes:requestMicrophoneAccess'),
readFileDataUrl: filePath => ipcRenderer.invoke('hermes:readFileDataUrl', filePath),
Expand Down
Loading
Loading