fix(sync): stop probing macOS protected folders during discovery - #1366
Conversation
Local project-identity discovery resolved every session's recorded working directory and read Git metadata from it, and the source-project probe stat-ed the same path. On macOS both reach into locations guarded by a TCC consent prompt, so a first sync raised a prompt for every guarded folder any session had ever run in. The desktop app requests no file-access entitlement; the prompts came from this passive access. Discovery now skips working directories under Desktop, Documents, Downloads, Movies, Music, Pictures, Library/CloudStorage, Library/Mobile Documents, and Dropbox. Sessions there keep path-only project identity and lose only Git remote, worktree, and branch detail. The new scan_protected_paths config option opts back in for users who keep code in those folders and accept the prompt. The gate lives on the engine and defaults to closed, so an engine built without the option, including the one that drives the startup identity backfill, cannot prompt. Closes #1364
roborev: Combined Review (
|
Review of the previous commit found two gaps. First, project extraction itself probes the recorded working directory: findGitRepoRoot stats every ancestor, reads .git file contents, lists sibling directories, and execs git, all before the engine's identity gates run, so parsing a session recorded under Documents still raised the consent prompt. Extraction now consults the same protected-path policy and falls back to the path basename for refused cwds. The guard is package-level because parsers run deep inside per-format code; NewEngine enables it when scan_protected_paths is set and never disables it. Second, the protected-path check compared lexically, so a working directory that reaches a protected folder only through a symlink passed the gate and the subsequent Stat or EvalSymlinks followed the link in. ResolvesIntoProtectedUserDataPath resolves one component at a time, checking each candidate lexically before touching it with Lstat, so answering the question never enters a protected location. Unresolvable links count as protected; home is also compared in symlink-resolved form so a home behind a linked ancestor still matches.
roborev: Combined Review (
|
The symlink-aware protected-path resolver Lstats each path component, and in the identity-cache gate it runs before the automount rejections inside NormalizeRootPath and discoverLocalGitIdentity. A locally attributed session with a /home/... cwd would therefore wake automountd on every one-minute cache expiry, the CPU storm those rejections exist to prevent. The resolver now refuses automounter namespaces at every resolution step, so both a literal /home/... input and a symlink hopping into the namespace mid-walk stop before any Lstat. Automount paths are reported unprotected: nothing there is user data, and downstream identity capture already rejects them itself.
roborev: Combined Review (
|
Review of the previous commits found the protected-path policy compressed two distinct hazards into one boolean and vetted only the cwd. The resolver now classifies a path as safe, protected user data, or automounter namespace. The automount class stays refused under the scan_protected_paths opt-in — consenting to consent prompts is not consenting to waking automountd — and a symlink hopping into /home is refused where the lexical checks cannot see it. The parser guard keeps one nuance: literal automount cwds already pass isForeignOSPath's resolved-autofs probe before the guard runs, so only symlink-discovered namespace paths are refused there. Git discovery also vets what gitfile contents point at. A linked worktree in an unguarded directory can name a gitdir or common directory inside a protected folder; identity capture and the parser previously read commondir, config, or HEAD there, and the parser could escalate to exec git against the same target. Both now abort at the worktree with path-only results when a target is refused. Enabling scan_protected_paths applies to sessions parsed afterward; docs now state that agentsview sync --full reparses existing sessions.
roborev: Combined Review (
|
Review of the previous commit found two remaining probe leaks. First, gitFileTargetsProbeable reused the cwd guard, whose automount allowance exists because isForeignOSPath vets literal cwds with the resolved-autofs probe before the guard runs. Gitfile targets never get that vetting, so a gitdir under /home reached readCommonDir and woke automountd. Targets now use their own guard that refuses automount namespaces outright. Second, both discovery paths vetted directories but statted and read the .git entry itself before classifying it. A .git symlink into a protected folder — a real pre-gitfile redirection pattern — was followed by the type probe, and the engine then read HEAD and config through it. Both paths now vet the exact .git path first; classification follows links, so the symlink case is refused without touching the target. Ancestor stats in the git-root walkers stay unvetted by choice: every read is now behind a vet, per-level classification would cost a quadratic Lstat walk on the hot parse path, and stat-only metadata access is not an established TCC trigger.
roborev: Combined Review (
|
Review of the previous commit found three remaining probe paths, and Windows CI failed on tests that drive the darwin classifier with POSIX fixtures. Missing-cwd sibling recovery read sibling gitfiles and their commondir targets without the gitfile-target vetting the upward walk applies, and verified deleted worktrees by listing a .git/worktrees directory derived from those targets. Sibling .git entries now go through the same Lstat-first typing and target vetting, so a refused sibling is skipped instead of recovering the protected main repository's name. Classification resolved the home directory with EvalSymlinks before any automount check, so a home under /home, or linked through /net, woke automountd on every call. Home resolution now walks component-by- component, aborts on any automounter candidate, and is memoized per process since home never changes. Both git-root walkers statted .git entries with a following stat before any vet. They now Lstat first and follow only symlinks whose target passes the guard; a refused link marks a repo boundary without a conservative result, so the parser cannot escalate to exec git against the same target. The four tests that exercise the darwin classifier's component walk with symlinks or literal /home paths now skip on Windows, where those fixtures are not absolute paths; production Windows behavior is unchanged because the classifier is inert off darwin.
roborev: Combined Review (
|
The lint CI job failed on nilaway: findLocalGitRoot mixed Lstat and Stat results in one flow, and statGitEntry could return a nil info with a nil error. The engine walker now types the entry in a helper where every dereference sits under its own error check, and statGitEntry signals a refused symlink with a sentinel error so info is non-nil exactly when err is nil. This worktree also had no prek hooks installed, which is how the failing commit got pushed; hooks are now installed so lint gates commits again. Review of the previous commit found two remaining gaps, both fixed. The ancestor boundary check in missing-cwd sibling recovery statted dir/.git with a following stat before any vet; it now types the entry through statGitEntry so a refused symlink counts as a boundary without being followed. And the exact metadata-file paths - HEAD, config, and commondir - are now vetted before reading: they sit inside vetted directories, but as symlinks they can lead into a protected folder, and reading through one would raise the prompt every directory-level vet already prevented.
roborev: Combined Review (
|
Review of the previous commit found that missing-cwd sibling recovery typed each sibling's .git entry before any vet. When the first existing ancestor is the home directory, the siblings include Documents and the other guarded folders, so typing them Lstats inside a guarded folder - and a guarded sibling holding a real .git directory flowed into deletedChildIsWorktree, whose ReadDir of the worktrees list is exactly the enumeration macOS gates behind a consent prompt. Each sibling's .git path is now vetted before statGitEntry touches it. For guarded siblings the lexical check answers without any filesystem access, so recovery from a deleted direct child of home skips Documents entirely instead of probing it.
roborev: Combined Review (
|
Review found that classification missed the physical spelling of user data paths. Since Catalina the writable system firmlinks user data under /System/Volumes/Data, so /System/Volumes/Data/Users/me/Documents is the same TCC-protected folder as ~/Documents and /System/Volumes/Data/home is the autofs home map. Firmlinks are not symlinks - Lstat reports a plain directory - so the classifier's link walk could not equate the two spellings and classified physical paths as safe. Both predicates now strip the data-volume prefix lexically before comparing, on the path and on home, without touching the filesystem. Every caller inherits the fix: classification entry checks, per- candidate walk checks, automount-safe home resolution, and the engine's pre-existing automount rejections.
roborev: Combined Review (
|
Review found two gaps in the probe classifier. Symlink splices and the input splitter collapsed ".." lexically via filepath.Join and filepath.Clean, before earlier components were resolved. The kernel resolves components in order, so home/q/../x with q linked into Documents classified as home/x while real resolution reaches Documents/x; a chained relative target like "l2/../safe" hid the same divergence. The walks now process raw components and resolve ".." as Dir(current) - correct because current never contains a symlink - and splices concatenate without cleaning so the restarted walk sees dotdot in traversal order. A dotdot path through a guarded midpoint now classifies as that midpoint, which is the conservative side of the traversal question. The automount namespace check and the APFS data-volume prefix trim compared case-sensitively, but the startup volume is case-insensitive by default, so /HOME/x or /system/volumes/data spellings bypassed them. Both now fold case; the protected-folder predicate already did.
roborev: Combined Review (
|
Review found two reads that could still enter guarded storage. Deleted-worktree verification vets the sibling's .git entry, but the worktrees directory inside a real .git can itself be a symlink, and ReadDir through it is exactly the enumeration macOS gates behind a consent prompt. The exact worktrees path is now vetted before enumerating, matching the policy every other metadata read follows. Source-project reconciliation gates the session cwd, but when the cwd is unavailable it resolved the durable snapshot's root with EvalSymlinks unconditionally. Snapshot roots are stored data that can predate protected-path gating and name a guarded folder, so a refused root now falls back to lexical containment instead of filesystem resolution. EvalSymlinks in prefix resolution moved behind a seam so the test can pin that a guarded root is never walked.
roborev: Combined Review (
|
Review found two remaining bypasses. Classification stopped at the first protected candidate, so a symlink inside ~/Documents leading into /home classified as protected, which the scan_protected_paths opt-in maps to probeable - violating the invariant that the opt-in lifts consent prompts, never automountd wakeups. The classifier now takes the caller's opt-in: with it set, the walk keeps resolving through protected prefixes (Lstat there is what the caller is about to do anyway) and still refuses automount targets; without it, the walk stops at the protected prefix untouched as before. A gitfile target without a commondir - the submodule layout - was considered probeable without vetting the gitdir's own config, and the conservative result escalates to gitMainRoot, whose git exec reads config and HEAD. Both are now vetted exactly when commondir is absent, treating the gitdir as the effective common directory.
roborev: Combined Review (
|
Review found that the cwd guard's automount allowance drifted out from under its justification. The allowance defers to isForeignOSPath, whose resolved-autofs probe matches the mount table's canonical prefixes case-sensitively with no data-volume trimming - but the allowance tested the broad classifier predicate, which since gained the /System/Volumes/Data spelling and case folding. Those alternate spellings bypassed the autofs probe entirely yet inherited its clearance, letting the git walk stat them and wake automountd. The allowance now uses IsCanonicalAutomountNamespacePath, which accepts only the exact spellings isForeignOSPath examines; alternate spellings and symlink-smuggled paths stay refused.
roborev: Combined Review (
|
Review found the automount allowance still trusted spelling over evidence. A canonically spelled cwd was admitted on the assumption that isForeignOSPath's probe had vetted it, but an exact namespace root like /home matches no trailing-separator prefix and was never probed, and a network home under /home carries the user's own guarded folders - Documents inside an autofs home bypassed protected-folder gating entirely because automount classification wins before the protected check. The allowance is now automountCwdProbeAllowed, which requires everything explicitly: canonical spelling, a path outside the network home's guarded folders unless scan_protected_paths is set, a non-root path, and - for autofs-managed prefixes - a first-level probe that actually resolved, consulted directly through the memoized probe cache shared with isForeignOSPath instead of assumed from call ordering. Namespaces not in the mount table stay probeable: there is no automountd behind them to wake.
roborev: Combined Review (
|
Review found the classifier only knew the fixed namespaces (/home, /net, /Network/Servers) while the parser discovers arbitrary autofs mounts such as /corp/home from the live mount table. A symlinked cwd, sibling, or gitfile target landing in a custom mount classified as safe, so Lstat walked inside it - the automountd wakeup the fixed namespaces already prevent. The engine's automount rejections had the same fixed-list gap. The parser now registers discovered prefixes with the classifier at detection time, since only the parser can discover them. Both predicates consult the registered set: the broad one with case folding and data-volume trimming, the canonical one against the mount table's exact form. Clearance for direct working directories is unchanged - automountCwdProbeAllowed already iterates the parser's prefix list, so a resolving first-level entry in a custom mount stays probeable and the mount root stays refused.
roborev: Combined Review (
|
Review found the last unvettable access path: the conservative gitfile fallback escalated to gitMainRoot, which execs git, and git follows config-derived paths - [include] path, includeIf, core.worktree - into locations no probe policy examined. That cannot be closed by vetting short of reimplementing git's config resolution, so passive discovery no longer runs the git binary at all. The pure-file logic already resolves every real layout: linked worktrees via commondir, bare-backed worktrees via core.bare, and the worktrees marker fallback. What the exec uniquely rescued - virtual repos via GIT_DIR environment setups and external gitdirs without commondir - now falls back to the directory basename, which is the path-only naming the protected-path work already established for refused locations. The two tests that pinned the git fallback now pin its absence: the shim would resolve the repository, so a reintroduced exec is caught by both the extracted name and the invocation log. The context parameter of ExtractProjectFromCwdWithBranchContext is retained for compatibility but no longer used.
roborev: Combined Review (
|
1 participant
Closes #1364.
Problem
On first open, the macOS app asked for access to Documents, Downloads, and Dropbox. The app requests no special entitlements — the prompts came from background sync reading files inside those folders:
Fix
/home,/net).HEAD,config,commondir), deleted-session recovery, and stored snapshot roots...in real traversal order, folds case, and recognizes the/System/Volumes/Datafirmlink spelling — without itself entering protected folders or waking automountd./corp/home) count as automount too.gitbinary: git follows config-derived paths ([include] path) that no vetting can constrain. All git-layout resolution is filesystem-local; rare layouts only git could resolve now get path-based names.Opting back in
scan_protected_paths = trueinconfig.tomlto restore full git info for code kept in protected folders. macOS prompts once per folder.agentsview sync --fullto refresh existing ones.Known limits
$HOME(outside~/Library/CloudStorage) would still prompt.~/Dropboxfolder (not cloud-mounted) loses git info it didn't have to; the opt-in restores it.Where to look
internal/export/project_identity.go— the classifierinternal/sync/engine.go— identity capture gatesinternal/parser/project.go— project-name extraction gates🤖 Generated with Claude Code