Add session-level auto-approve for AI file read operations#3101
Open
mits-pl wants to merge 4 commits intowavetermdev:mainfrom
Open
Add session-level auto-approve for AI file read operations#3101mits-pl wants to merge 4 commits intowavetermdev:mainfrom
mits-pl wants to merge 4 commits intowavetermdev:mainfrom
Conversation
Adds an "Allow reading in this session" button to the AI tool approval UI. When clicked, all subsequent file reads from the same directory are auto-approved for the rest of the session (in-memory only, resets on restart). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
WalkthroughAdds a session-scoped read-approval flow for AI tool reads. Frontend: extracts directory targets from tool descriptions, conditionally shows an “Allow reading in this session” button for read operations, and calls WaveAIModel.sessionReadApprove. Backend: introduces an in-memory SessionApprovalRegistry with home-dir expansion, canonicalization, and sensitive-path checks; adds RPC types, server handler, and client wrappers for Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
frontend/app/aipanel/aitooluse.tsx (1)
205-212: Apply session approvals before sending final approve action.At Line 211 and Line 291,
handleApprove()runs immediately after firing session-approval RPCs. If those calls are delayed/fail, subsequent reads may still prompt unexpectedly despite the user choosing session allow.💡 Proposed fix
- const handleAllowSession = () => { + const handleAllowSession = async () => { const dirs = extractDirsFromParts(parts); const model = WaveAIModel.getInstance(); - for (const dir of dirs) { - model.sessionReadApprove(dir); - } + await Promise.allSettled(dirs.map((dir) => model.sessionReadApprove(dir))); handleApprove(); }; @@ - const handleAllowSession = () => { + const handleAllowSession = async () => { const dir = extractDirFromToolDesc(toolData.tooldesc); if (dir) { - WaveAIModel.getInstance().sessionReadApprove(dir); + await WaveAIModel.getInstance().sessionReadApprove(dir); } handleApprove(); };Also applies to: 286-292
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/aipanel/aitooluse.tsx` around lines 205 - 212, The handler handleAllowSession calls WaveAIModel.getInstance().sessionReadApprove(dir) in a loop then immediately calls handleApprove(), which can race with async RPCs; make handleAllowSession async, collect the promises returned by sessionReadApprove (via extractDirsFromParts(parts)), await Promise.all(promises) and only then call handleApprove(); apply the same change to the other similar handler that performs sessionReadApprove before handleApprove so approvals complete before sending the final approve action.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/aipanel/aitooluse.tsx`:
- Around line 80-94: The parent-directory extraction in extractDirFromToolDesc
only checks for '/'; change it to consider both '/' and backslash '\\' by
computing the last separator as the max of filePath.lastIndexOf('/') and
filePath.lastIndexOf('\\') and using that index to derive the parent directory
(preserving the existing special-case behavior for root/leading slash). Update
the logic around lastSlash to use this combined separator index so Windows paths
like "C:\..." return the correct parent directory.
In `@pkg/aiusechat/tools_readdir.go`:
- Around line 168-180: ToolApproval's auto-approval path for read_dir (in the
ToolApproval function that calls parseReadDirInput, wavebase.ExpandHomeDir and
IsSessionReadApproved) lacks the blocked-path guard used by read_text_file, so
session approvals can let protected dirs (e.g. ~/.ssh, ~/.aws or their parent
roots) be enumerated; modify ToolApproval to run the same blocked-path check
used by read_text_file (the verifier that enforces forbidden roots/paths) after
expanding the path and before returning ApprovalAutoApproved, and ensure the
check treats directory roots and parent paths as blocked (not just exact file
matches) so any path under a forbidden root is rejected and returns
ApprovalNeedsApproval.
In `@pkg/aiusechat/tools_readfile.go`:
- Around line 405-417: The current ToolApproval flow auto-approves files in
session-approved parent directories but isBlockedFile() only blocks a few exact
filenames and key-like names, so files under sensitive dirs (e.g., ~/.ssh/*,
~/.aws/*) still get auto-approved; update the isBlockedFile helper to reject
paths that have those directory prefixes (after expanding/home-normalizing the
path) so files like ~/.ssh/config and ~/.aws/config are denied even if their
parent dir is session-approved—locate isBlockedFile and ensure it checks
normalized directory prefixes (e.g., ".ssh", ".ssh/", ".aws", ".aws/") and
returns blocked before ToolApproval/IsSessionReadApproved is consulted, using
the same normalization logic as parseReadTextFileInput/wavebase.ExpandHomeDir.
---
Nitpick comments:
In `@frontend/app/aipanel/aitooluse.tsx`:
- Around line 205-212: The handler handleAllowSession calls
WaveAIModel.getInstance().sessionReadApprove(dir) in a loop then immediately
calls handleApprove(), which can race with async RPCs; make handleAllowSession
async, collect the promises returned by sessionReadApprove (via
extractDirsFromParts(parts)), await Promise.all(promises) and only then call
handleApprove(); apply the same change to the other similar handler that
performs sessionReadApprove before handleApprove so approvals complete before
sending the final approve action.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1bb03a68-f057-48d5-aaf5-c5eb0e07b77b
📒 Files selected for processing (10)
frontend/app/aipanel/aitooluse.tsxfrontend/app/aipanel/waveai-model.tsxfrontend/app/store/wshclientapi.tsfrontend/types/gotypes.d.tspkg/aiusechat/sessionapproval.gopkg/aiusechat/tools_readdir.gopkg/aiusechat/tools_readfile.gopkg/wshrpc/wshclient/wshclient.gopkg/wshrpc/wshrpctypes.gopkg/wshrpc/wshserver/wshserver.go
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sensitive paths (~/.ssh, ~/.aws, ~/.gnupg, ~/.kube, ~/.password-store, ~/.secrets, keychains, Windows credentials) are now excluded from session auto-approval. Even if a parent directory like ~ is session-approved, reads from these directories still require manual approval. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/aiusechat/sessionapproval.go`:
- Around line 29-42: The stored approved path must be fully canonicalized
(absolute + symlink-resolved and normalized case on Windows) before inserting
into globalSessionApproval.approvedPaths and candidate paths must be
canonicalized before any prefix checks; update AddSessionReadApproval to call
filepath.Abs then filepath.EvalSymlinks (fall back to the Abs/cleaned value only
if EvalSymlinks returns an error), normalize case on Windows (e.g.,
strings.ToLower), ensure a trailing filepath.Separator, and store that canonical
string in globalSessionApproval.approvedPaths; also update the code that checks
approvals (the code paths invoked by WaveAISessionReadApproveCommand) to apply
the same canonicalization to the requested path before comparing prefixes and
handle canonicalization errors by denying approval or returning a clear error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4cb7cfea-2255-44b5-93eb-6d605216c446
📒 Files selected for processing (1)
pkg/aiusechat/sessionapproval.go
Use filepath.EvalSymlinks() on both stored approval roots and candidate paths to prevent symlink-based bypass of session approval boundaries. Also reject sensitive directories at registration time in AddSessionReadApproval. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
pkg/aiusechat/sessionapproval.go (1)
62-99:⚠️ Potential issue | 🟠 MajorNormalize both sides before sensitive/prefix checks (case + canonical roots).
Line 67/Line 83 and Line 115 compare path strings directly, but only the candidate path is canonicalized. Sensitive roots and approval roots are not normalized to the same comparison form (notably case on Windows, and alias/symlink variants of protected roots), so sensitive-path detection and approval matching can be bypassed or behave inconsistently.
Suggested fix
import ( "os" "path/filepath" + "runtime" "strings" "sync" @@ func canonicalizePath(rawPath string) string { @@ return resolved } + +func normalizePathForCompare(rawPath string) string { + p := filepath.Clean(canonicalizePath(rawPath)) + if runtime.GOOS == "windows" { + p = strings.ToLower(p) + } + return p +} @@ func AddSessionReadApproval(dirPath string) { - canonical := canonicalizePath(dirPath) + canonical := normalizePathForCompare(dirPath) @@ func isSensitivePath(expandedPath string) bool { - homeDir := os.Getenv("HOME") - if homeDir == "" { - homeDir = os.Getenv("USERPROFILE") - } - cleanPath := filepath.Clean(expandedPath) + homeDir, _ := os.UserHomeDir() + cleanPath := normalizePathForCompare(expandedPath) @@ for _, dir := range sensitiveDirs { - dirWithSep := dir + string(filepath.Separator) - if cleanPath == dir || strings.HasPrefix(cleanPath, dirWithSep) { + normalizedDir := normalizePathForCompare(dir) + dirWithSep := normalizedDir + string(filepath.Separator) + if cleanPath == normalizedDir || strings.HasPrefix(cleanPath, dirWithSep) { return true } } @@ if localAppData := os.Getenv("LOCALAPPDATA"); localAppData != "" { - credPath := filepath.Join(localAppData, "Microsoft", "Credentials") + credPath := normalizePathForCompare(filepath.Join(localAppData, "Microsoft", "Credentials")) if cleanPath == credPath || strings.HasPrefix(cleanPath, credPath+string(filepath.Separator)) { return true } } if appData := os.Getenv("APPDATA"); appData != "" { - credPath := filepath.Join(appData, "Microsoft", "Credentials") + credPath := normalizePathForCompare(filepath.Join(appData, "Microsoft", "Credentials")) if cleanPath == credPath || strings.HasPrefix(cleanPath, credPath+string(filepath.Separator)) { return true } } @@ func IsSessionReadApproved(filePath string) bool { - canonical := canonicalizePath(filePath) + canonical := normalizePathForCompare(filePath)#!/bin/bash set -euo pipefail echo "Inspect current comparison normalization in session approval logic:" sed -n '27,125p' pkg/aiusechat/sessionapproval.go echo echo "Check whether case normalization is currently present:" rg -n 'ToLower|EqualFold|runtime\.GOOS' pkg/aiusechat/sessionapproval.go echo echo "Check whether sensitive roots are canonicalized before comparison:" rg -n 'sensitiveDirs|isSensitivePath|canonicalizePath\(|normalizePathForCompare' pkg/aiusechat/sessionapproval.goAlso applies to: 107-116
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/aiusechat/sessionapproval.go` around lines 62 - 99, The isSensitivePath logic compares only the candidate path after filepath.Clean, which allows mismatches on case-insensitive OSes and symlink/alias variants; update isSensitivePath (and the related approval-root checks around lines referenced) to canonicalize and normalize both sides before comparing: resolve abs + EvalSymlinks (or fall back to Clean), normalize separators, and on Windows use case-insensitive comparison (strings.EqualFold) or strings.ToLower for both the cleaned expandedPath and each sensitiveDir/approvalRoot; ensure you canonicalize every entry in sensitiveDirs (and the LOCALAPPDATA/APPDATA derived credPath) prior to the equality/HasPrefix checks and use the same dirWithSep normalization for prefix checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@pkg/aiusechat/sessionapproval.go`:
- Around line 62-99: The isSensitivePath logic compares only the candidate path
after filepath.Clean, which allows mismatches on case-insensitive OSes and
symlink/alias variants; update isSensitivePath (and the related approval-root
checks around lines referenced) to canonicalize and normalize both sides before
comparing: resolve abs + EvalSymlinks (or fall back to Clean), normalize
separators, and on Windows use case-insensitive comparison (strings.EqualFold)
or strings.ToLower for both the cleaned expandedPath and each
sensitiveDir/approvalRoot; ensure you canonicalize every entry in sensitiveDirs
(and the LOCALAPPDATA/APPDATA derived credPath) prior to the equality/HasPrefix
checks and use the same dirWithSep normalization for prefix checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 102c550f-4cb5-4391-8183-5281b3706968
📒 Files selected for processing (1)
pkg/aiusechat/sessionapproval.go
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
3 participants
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.
Summary
Changes
pkg/aiusechat/sessionapproval.go— thread-safe in-memory registry for session-approved pathstools_readfile.go/tools_readdir.go—ToolApprovalchecks session allowlist before requiring manual approvalWaveAISessionReadApproveCommandto register approved paths from frontendSecurity
.ssh,.aws, credentials) inisBlockedFile()are enforced viaToolVerifyInputbeforeToolApproval— session approval cannot bypass themTest plan
~/.ssh/id_rsa) are still blocked even with session approval🤖 Generated with Claude Code