mirror of
https://github.com/stablyai/orca.git
synced 2026-10-03 00:02:19 +00:00
Jinwoo HongandOrca
ea9a718e29
When a remote SSH workspace contains a symlink whose target lies outside
the registered repo/worktree roots, file reads failed with 'Path outside
authorized workspace'. This silently broke common workflows: HPC dataset
mounts, multi-checkout repos, dotfile editing, and any cross-mount
symlink.
Drop `RelayContext.authorizedRoots`, `validatePath`, and
`validatePathResolved` along with all ~33 call sites in fs-handler.ts
and git-handler.ts. The relay's threat model becomes 'the relay runs as
the SSH user and trusts the renderer.'
Why this is acceptable: `pty.spawn` and `git.exec` already concede the
same threat. A renderer that wants to reach `/etc/passwd` can spawn a
shell or run `git -C /etc cat-file`; the FS allowlist was friction, not
a security boundary. Intra-worktree path checks in `getDiff` and
`discard` are intentionally preserved.
Back-compat preserved: `session.registerRoot` (notification + request)
remains a valid RPC, retained as no-ops on new relays. Old main + new
relay and new main + old relay both keep working through the upgrade
window. `registerRelayRoots` is also kept for the same reason. A
narrowed error-translation block in `worktree-remote.ts` handles old
relays still surfacing the legacy error string to users.
Tests: removed two negative-allowlist tests; added a positive control
('reads files outside any registered root') and a direct regression
test for #1661 ('reads files via symlinks resolving outside the
workspace'). All 469 relay/SSH/IPC tests pass.
See docs/relay-fs-allowlist-removal.md for the full rationale,
back-compat matrix, alternatives considered, and follow-up cleanup
plan.
Closes #1661
Co-authored-by: Orca <help@stably.ai>
32 lines
1.2 KiB
TypeScript
32 lines
1.2 KiB
TypeScript
import { resolve } from 'path'
|
|
import { homedir } from 'os'
|
|
|
|
// Why: Node's fs APIs don't understand shell tilde expansion. Old repos may
|
|
// have been stored with `~` or `~/…` paths before the client-side fix, so the
|
|
// relay must expand them to absolute paths as a safety net.
|
|
export function expandTilde(p: string): string {
|
|
if (p === '~' || p === '~/') {
|
|
return homedir()
|
|
}
|
|
if (p.startsWith('~/')) {
|
|
return resolve(homedir(), p.slice(2))
|
|
}
|
|
return p
|
|
}
|
|
|
|
// Why: the relay runs as the SSH user and trusts the renderer process. A
|
|
// compromised renderer can already weaponize pty.spawn and git.exec to reach
|
|
// any path the SSH user can reach, so the FS-side allowlist provided friction
|
|
// without meaningfully narrowing the blast radius. See
|
|
// docs/relay-fs-allowlist-removal.md.
|
|
//
|
|
// registerRoot is retained as a no-op so existing session.registerRoot RPC
|
|
// calls (notification + request) remain valid during the relay-deploy upgrade
|
|
// window where an old main may still call into a new relay (and vice versa).
|
|
// Tracked for deletion once the relay-version floor moves past the cutover.
|
|
export class RelayContext {
|
|
registerRoot(_rootPath: string): void {
|
|
// intentionally empty
|
|
}
|
|
}
|