Repository navigation
refactor(files): one access resolver for desktop and host file requests - #28196
Closed
brennanb2025 wants to merge 3 commits into
Closed
brennanb2025 wants to merge 3 commits into
brennanb2025 wants to merge 3 commits into
Conversation
The desktop IPC resolver hardwired its default-root check (project roots plus the floating-workspace folder) while the host's runtime file services called the bare store-root check directly. Name the root set as a LocalFileRootScope: desktop adds the floating folder, the host adds nothing. resolveLocalRequestPathInScope takes the scope, the desktop entry points pass the desktop scope, and every runtime file caller (and the local filesystem provider it injects) uses resolveHostAuthorizedPath. No behaviour change.
…onours The host scope inherited the desktop's declared-kind rules, so a host request declaring user-file would have reached any path. Each scope now lists the kinds it honours: the desktop all current kinds, the host none yet. An unhonoured kind gets the root check only.
7 of 9 tasks
…aches The chat-image row used a markdown file that chat-image refuses anyway, so it could not catch a regression; each row now targets a path the desktop allows through that kind.
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ELI5
Orca checks every file request against a list of folders the request is allowed to touch. Two parts of Orca keep that list separately: the desktop window (your projects plus the app's floating-workspace folder) and the file service a paired computer, phone, or SSH session reaches through Orca's server (your projects only). This PR gives both one shared checker and passes each caller's folder list in. Nothing you see changes.
What Changed
The problem. No bug is fixed here; this is plumbing. Later PRs in this stack let a paired or SSH host open files in its Orca scratch folder (where chats write HTML visuals) by absolute path. That host-side request has to be authorized by the same rules the desktop already uses for files opened by name, chat images, and document images. Today those rules live in one desktop-only resolver whose folder list is hardwired. The host's runtime file code calls the bare project-folder check directly, in five files plus the injected local filesystem provider. Extending the host would have meant a second copy of the resolver.
User-facing change. None. Every caller gets the same folders as before:
Mechanism.
New
src/main/ipc/local-file-root-scope.tsnames what a caller may reach.LocalFileRootScopeis{ extraRoots(): readonly string[]; accessKinds: ReadonlySet<kind> }:extraRoots(): the folders a caller adds beyond the store's registered roots;accessKinds: the declared access kinds the resolver honours outside the roots for that caller.It defines two scopes,
resolveScopedAuthorizedPath(path, store, scope, options)(the default-root check) andresolveHostAuthorizedPath:DESKTOP_FILE_ROOT_SCOPE: adds the floating folder and honours every current kind;HOST_FILE_ROOT_SCOPE: adds nothing and honours no kind yet.local-file-access-resolution.ts: the resolver is nowresolveLocalRequestPathInScope(path, access, store, operation, scope). Both its default check and the chat-image network-share check use the passed scope. A declared kind that the scope does not honour is treated like no declared access: root check only.resolveLocalRequestPathandresolveDesktopAuthorizedPathkeep their signatures and pass the desktop scope, so existing desktop callers and tests are untouched.Runtime host side: the local filesystem provider and the runtime file commands now call
resolveHostAuthorizedPathinstead of the bare check. The commands covered are explorer preview and chunk reads, phone file reads, watcher roots, and no-clobber upload commits.Stack position. S1 of 14 in the chat-visuals scratch-folder stack. Base
main; no parent PR. S3 (host{path, access?}file targets) builds on it. It authorizes those requests throughresolveLocalRequestPathInScopewith the host scope, and extends that one value: this host's chat-visuals folder as a root, anduser-fileconfined to the host roots. On the host a clicked file never reaches an arbitrary path the way it can on the desktop. Later, S4 adds thechat-visualkind there.Why
Differences from the common pattern:
user-file.Linked Issue
N/A, maintainer stack PR (chat-visuals scratch-folder plan, PR 2 of the signed plan).
Visual Proof
N/A: no visual or interaction change; authorization refactor with identical root sets.
Testing
local-file-access-resolution.test.tsandlocal-file-access-resolution-network-path.test.ts(the existing resolution matrix) pass unchanged.New
local-file-root-scope.test.ts(real filesystem):user-filereaches outside the roots on the desktop only. On the host every kind (user-file read/write, chat image, document resource, document folder) is refused outside the roots and still allowed inside them;New
local-file-root-scope-windows-path.test.ts: Windows path forms in both scopes, with Node's path module switched to win32 on every host. Covers drive-letter case, forward slashes,.., a folder workspace on another drive with a space in its name, sibling-prefix and cross-drive refusals, and the floating folder (desktop only).orca-runtime-files-terminal-link-host-translation.test.ts: one assertion now states the host scope's empty extra roots, since the mocked check receives them.Every test under
src/main/{runtime,ipc,providers,window,media}passes locally: 1774 files, 18129 tests.tsc -p config/tsconfig.node.json,pnpm run typecheckand the changed-code quality gate pass.Platforms: macOS locally; Windows path forms through the win32 path test; CI covers Linux.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
Review
Agent skill upstream boundary
Notes
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)