feat(files): permission-gated files:read scoped to declared globs#456
Conversation
Adds the files:read permission (Xoshbin#448): a bounded text read for Tier-2 extensions, routed through the same double-layer gate as fs:watch and scoped to the glob patterns declared in permissionArgs["files:read"]. - New files_scope.rs: pattern validation (non-empty, no '..', valid glob — deliberately NOT fs:watch's $HOME anchor, so patterns may target other drives or be unanchored like **/steamapps/appmanifest_*.acf), GlobSet coverage check (case-insensitive on Windows), and a hard deny-list of credential stores and OS dirs that beats even consented patterns. - New files_read_text command: permission check, declared-globs coverage on the normalized path, deny-list on both the normalized and the canonicalized (symlink-resolved) path, 1 MiB read ceiling. The fixed home/app-data/temp roots are NOT unioned into the extension scope; the launcher's own app-data dir joins the deny-list. Host-context callers keep read_text_preview semantics. - Manifest load-time validation generalized: 'declared iff args present, args are a validated string array' now covers both fs:watch and files:read. - files namespace joins ALWAYS_INJECTS_CALLER_ID so the router-verified caller identity reaches the Rust command; SDK IFilesService gains read(path, { maxBytes }), which rejects with the denial reason rather than resolving empty. - Consent surface (PR Xoshbin#455) renders the declared globs with no extra work: permission catalog entry + docs added. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV7sYU38w7yYUyuQgioobk
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV7sYU38w7yYUyuQgioobk
VALID_PERMISSIONS never learned files:search when Xoshbin#444 introduced it, so `asyar validate` rejects manifests the launcher happily loads; files:read joins the same list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV7sYU38w7yYUyuQgioobk
There was a problem hiding this comment.
Code Review
This pull request introduces a new permission-gated file reading API (files:read) that allows extensions to read file contents within declared glob patterns, while excluding credential stores and OS directories. The feedback highlights two critical security vulnerabilities: first, a symlink bypass where the resolved canonical path is not re-verified against the allowed patterns and the deny-list paths are not canonicalized; second, a case-insensitive path bypass on macOS due to case-sensitive prefix matching on that platform.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…OS case Addresses the bot review on Xoshbin#456, plus one bug it missed: - Re-check glob coverage (not just the deny-list) against the canonical path, so a symlink inside a covered glob can't read an arbitrary uncovered file. Deny runs first so laundering into a protected root reports the specific error. - Canonicalize the comparison anchors (home, extra deny roots) for the post-resolution checks; a symlinked home or app-data dir no longer skews them. Entries that don't exist keep their literal form. - Swap Path::canonicalize for dunce::canonicalize: Windows canonicalization yields verbatim \\?\C:\ paths whose prefix component never starts_with-matches a normal-form root and never glob-matches an anchored pattern — the previous canonical-side deny check was a no-op on Windows (the bot missed this; junctions need no privileges there). - Extend case-insensitive matching (deny prefix check AND coverage globs) to macOS: APFS is case-insensitive by default and the deny list fails open on a case mismatch. Known trade, called out in the PR thread: with coverage re-checked on the canonical path, macOS patterns anchored at /tmp must be declared in /private form (or unanchored); fs:watch avoids this by never re-checking coverage, which is exactly the hole being closed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV7sYU38w7yYUyuQgioobk
…ile manager operations
|
Hey @dose-dot-dev Thanks for this — really thorough work. The symlink handling stood out in particular: double-checking both coverage and the deny-list before and after canonicalization (and using dunce to sidestep the Windows verbatim-prefix issue) closes exactly the kind of laundering hole this sort of feature usually ships with, and having dedicated tests for the symlinked-home case shows you actually tried to break it, not just made it work. Appreciate that it slots into the existing architecture cleanly too — enforcement staying in Rust, the consent UI picking up the new glob patterns with zero UI changes, and reusing the fs:watch double-layer gate pattern rather than inventing a new one. Nice work overall. |
Closes #448. Adds the
files:readpermission: a bounded text read for Tier-2 extensions, scoped to the glob patterns declared inpermissionArgs["files:read"], routed through the same double-layer gate asfs:watchand rendered by the consent surface from #455 with no extra UI work.Built to the constraints from the issue thread:
How it works
validate_path_allowedare never unioned in — a Steam extension declaringC:/Program Files (x86)/Steam/**cannot read~/.sshfor free. Host-side callers ofread_text_previeware untouched.files_scope.rs): credential stores (~/.ssh,~/.aws,~/.gnupg,~/.kube, keychains, Windows DPAPI/Credential Manager dirs, …), OS locations (/etc,/proc,%SystemRoot%, …), and the launcher's own app-data dir (settings.dat holds consent records and MCP config — a consented~/**shouldn't read the launcher's internal state).files_read_textcommand checks: permission → glob coverage on the normalized path → deny-list → canonicalize (resolves symlinks) → deny-list again on the real location, so a covered symlink can't launder a read out of a protected dir. Reads clamp to a 1 MiB ceiling (default stays at the preview's 50 KB) and reuseread_text_preview's bounded-read primitive: byte cap, lossy UTF-8,normalize_path.fs:watchas discussed: patterns keep the non-empty / no-../ valid-glob checks but drop the$HOME-anchor rule — anchoring on another drive, or nowhere at all, is the point. On Windows, coverage matches case-insensitively (NTFS semantics).globsettreats a leading**/as "any absolute prefix" (drive letters included, after backslash normalization), so the single static pattern**/steamapps/appmanifest_*.acfcovers arbitrary, user-configured library drives. Pinned by a dedicated test (unanchored_pattern_covers_any_drive) so it can't regress silently. And Steam'slibraryfolders.vdflists every installed appid per library, so the crawl needs no directory enumeration — direct reads by path suffice.IFilesService.read(path, { maxBytes }), which rejects with the denial reason rather than resolving empty (a denial must be distinguishable from an empty file). Thefilesnamespace joinsALWAYS_INJECTS_CALLER_IDso the router-verified caller identity reaches the Rust command. Also folds in a one-line pre-existing gap: the CLI'sVALID_PERMISSIONSnever learnedfiles:searchwhen Feat/file search #444 introduced it, soasyar validaterejected manifests the launcher happily loads; bothfiles:*entries are added.Judgment calls I'd flag for review: the 1 MiB ceiling, the deny-list contents, and including the launcher's own app-data dir in it.
Verification
cargo test(2530+ passed; the one failure is the knownfs_watcher::registry_tests::rejects_when_global_total_paths_would_exceed_limitunder WSL's inotify limits, reproduces on cleanmain),cargo clippy --all-targets -- -D warningsclean,cargo fmtclean,pnpm -r test:rungreen (~30 new tests: pattern validation, cross-drive coverage, deny-beats-glob, symlink escape,..normalization, size clamp, service/proxy layers),prettier --checkclean.C:/D:/B:) through the**/steamapps/appmanifest_*.acfpattern with no console flash; on a launcher without this PR the extension's probe fails closed and it falls back to its PowerShell path.🤖 Generated with Claude Code