Interactive file reads reach as far as shell does (#1724) - #1770
Open
Aaronontheweb wants to merge 10 commits into
Open
Interactive file reads reach as far as shell does (#1724)#1770Aaronontheweb wants to merge 10 commits into
Aaronontheweb wants to merge 10 commits into
Conversation
Interactive Personal-audience sessions now get shell-equivalent read and attach reach: file_read, file_list, attach_file resolve outside the configured trust roots, matching the approval-gated shell surface. This kills the shell-workaround (cat, cp-into-session) for legitimate out-of-roots files. ToolPathPolicy.IsReadDenied now also denies the shell indicator list (config dir, sqlite DB, pid, lock, restart manifest), so read tools cannot reach control-plane files that shell cannot even reference. Autonomous sessions, Team, and Public audiences keep their roots-scoped or fail-closed behavior. attach_file lifts its session-proximity gate only for interactive Personal; out-of-session files still copy into the session attachments dir. Spec: netclaw-tools carve-out for interactive Personal read reach.
![github-code-quality[bot]](https://avatars.github.com/u/9919?s=60&v=4){kind=small}
|
|
||
| public InteractivePersonalReadReachTests() | ||
| { | ||
| _sessionDir = Path.Combine(_dir.Path, "sessions", "s1"); |
| public InteractivePersonalReadReachTests() | ||
| { | ||
| _sessionDir = Path.Combine(_dir.Path, "sessions", "s1"); | ||
| _outsideDir = Path.Combine(_dir.Path, "outside"); |
| var ctx = Ctx(audience, autonomous: !interactive); | ||
|
|
||
| var path = outsideRoots | ||
| ? Path.Combine(_outsideDir, "notes.txt") |
|
|
||
| var path = outsideRoots | ||
| ? Path.Combine(_outsideDir, "notes.txt") | ||
| : Path.Combine(_sessionDir, "notes.txt"); |
| var ctx = Ctx(audience, autonomous: !interactive); | ||
|
|
||
| var path = outsideRoots | ||
| ? Path.Combine(_outsideDir, "report.png") |
|
|
||
| var path = outsideRoots | ||
| ? Path.Combine(_outsideDir, "report.png") | ||
| : Path.Combine(_sessionDir, "report.png"); |
| bool interactive, | ||
| bool expectedAttached) | ||
| { | ||
| var outsideFile = Path.Combine(_outsideDir, "report.png"); |
…ches (#1724) Adversarial review (PR #1770) found two blockers: 1. attach_file had no ToolPathPolicy, so the lifted proximity gate let interactive Personal attach secrets/keys/db/pid/lock — a full bypass of the read-deny surface. AttachFileTool now takes ToolPathPolicy and applies IsReadDenied after path resolution, matching file_read/list. 2. set_working_directory inherited shell-equivalent reach via the shared TryResolveReadPath, widening the safe-verb auto-approve zone and letting a planted AGENTS.md become system-prompt content. New TryResolveWorkingDirectory opts out of interactive Personal reach; SetWorkingDirectoryTool stays roots-scoped. Also: ToolPathPolicyTests fixture now includes ConfigDirectory in shell indicators to match production (Program.cs), and the ls regression test asserts the production behavior (denied). New tests cover attach deny of control-plane files and set_working_directory roots-scoping.
| // BLOCKER regression (#1724): attach must use the same hard-deny surface | ||
| // as file_read/file_list, so interactive Personal reach cannot ship | ||
| // secrets/keys/db/pid/lock that shell cannot even reference. | ||
| var secretsPath = Path.Combine(_outsideDir, "secrets.json"); |
…_working_directory in Mode.All (#1724) Second adversarial review found: 1. IsReadDenied bypass via symlinked INTERMEDIATE directory (ln -s config /tmp/x then read /tmp/x/netclaw.json). IsDeniedAgainst now resolves intermediate symlinks segment-by-segment via TryResolveSymlinksInPath, mirroring the shell scanner. attach_file re-checks the deny against the resolved path as defense-in-depth. 2. set_working_directory opt-out was inert for the default Mode.All Personal profile — the Mode.All branch fired before the opt-out flag was consulted. The branch now clamps set_working_directory to the autonomous zone in every mode. 3. Prefix-collision gap: sqlite sidecar files (wal/shm/journal) held raw page data (secrets) but path-boundary matching allowed reads while shell denied them. Program.cs shell indicator list now includes the sidecars; fixture mirrors production. 4. CredentialReadDenied message now covers control-plane state, not just credentials/keys; ToolPathPolicy remarks updated to match production (directory-scoped shell entries are intentional). 5. Spec: attach_file and set_working_directory requirements added. Tests: symlinked-dir read-deny regression, Mode.All working-dir clamp, Roots-mode attach branch (was untested), sidecar read-deny cases.
…ars on write (#1724) Verification review (approve-with-nits) flagged a stale scenario in openspec/specs/session-cwd/spec.md that promised set_working_directory allows any valid directory under Personal Mode.All — the opposite of the new autonomous-zone clamp. Updated the requirement + scenario to codify the clamp and cross-reference the interactive read reach. Also add sqlite sidecars to writeDenyList for defense-in-depth parity with the shell indicator list (read already denied via the union).
TryResolveSymlinksInPath built partial paths with a bare directory
separator, so on Windows every probe looked like '\Users\...' instead of
'C:\Users\...' and symlink resolution silently no-oped. This surfaced in
IsReadDenied_blocks_symlinked_directory_traversal on the Windows CI job:
the symlink was created but IsReadDenied returned false.
Path.GetPathRoot preserves the full root on every platform ('C:\' on
Windows, '/' on Unix), so the segment walk resolves symlinks correctly
everywhere. The shell scanner (CommandReferencesDeniedPath) shares this
function and was affected on Windows too.
The test IsReadDenied_blocks_symlinked_directory_traversal fails on Windows CI. We do not know the exact cause yet. This diagnostic shows the paths that IsReadDenied compares. It shows the raw link path, the resolved link target, the candidate path, and the denied directory. It shows the result of the StartsWith check. The message appears only when the assert fails. On Linux the assert passes, so the message stays hidden. This proves the diagnostic code compiles and does not throw.
The old diagnostic showed the two paths only. It did not show which segment of TryResolveSymlinksInPath diverges on Windows. The new diagnostic replays the segment walk. It logs, for each segment, the directory and file existence check, the ResolveLinkTarget outcome, and the rebuilt path. The message prints only when the assert fails.
TryResolveSymlinksInPath seeded the builder with the path root and then split the full path. On Windows the first split token is the drive, so the walk built "C:\C:\Users\..." and every existence probe missed. The symlink walk then no-oped and IsReadDenied failed open. Now the split covers only the remainder after the root. This also removes the temporary CI diagnostic block from the regression test for #1724.
| var policy = new ScopedFileAccessPolicy(config, _paths); | ||
| var ctx = Ctx(TrustAudience.Personal, autonomous: false); | ||
|
|
||
| var outside = Path.Combine(_outsideDir, "notes.txt"); |
| var policy = new ScopedFileAccessPolicy(new ToolConfig(), _paths); | ||
| var ctx = Ctx(TrustAudience.Personal, autonomous: false); | ||
|
|
||
| var outside = Path.Combine(_outsideDir, "notes.txt"); |
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
1 participant
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.
Interactive Personal-audience sessions get shell-equivalent read/attach reach.
Changes
file_read/file_list/attach_fileresolve outside trust roots for interactive Personal sessions (autonomous, Team, Public unchanged)ToolPathPolicy.IsReadDeniednow also denies the shell indicator list (config dir, sqlite DB, pid, lock, restart manifest) — read tools cannot reach files shell cannot even referenceattach_filelifts its session-proximity gate only for interactive Personal; out-of-session files still copy into the session attachments dirnetclaw-toolscarve-out for interactive Personal read reachInteractivePersonalReadReachTests(theory matrix), updatedAttachFileToolTests,ToolPathPolicyTestsCloses #1724