[AIR-Xie]

Summary

• require explicit approval before the Read tool or read-only Bash commands access environment files, SSH private keys, AWS credentials, and common credential/secret/token files
• route file arguments from tac, rev, fold, expand, unexpand, fmt, pr, comm, cmp, and tsort through the shared sensitive-read check
• inspect actual matches for filename globs, ask for directory-segment globs only when their final segment targets a sensitive name, and check both original and resolved symlink paths
• allow reads of existing directories named token or credentials while retaining checks for sensitive symlink names
• preserve safe reads including public keys, known_hosts, SSH config, shell profiles, .env.example, .env.sample, .env.template, .env.dist, .envrc, and .environment
• skip this mandatory prompt in bypass mode and plan mode entered from bypass; Permission guardrail: opt-in hosted decision model behind the rules #146 owns the opt-in bypass guardrail
• omit the ineffective directory allow suggestion from sensitive Bash read prompts
• recognize only the exact shell no-op : as read-only, based on train/dev/test data

Why

Read-only Bash commands could previously access project-local secrets without confirmation. The first implementation put the check in the shared filesystem permission layer, but review identified bypasses through commands without path extractors, wildcard patterns that only became sensitive after shell expansion, and symlinks whose original name was discarded after resolution.

Read and Bash now use one sensitive-path definition. Bash expands a wildcard in the final path segment against its directory, checks each match, and performs a second check against the original path after normal path validation succeeds. Permission-mode bypass semantics are centralized so unrestricted sessions remain unrestricted while default sessions keep a non-classifier-approvable safety prompt.

Prompt reduction remains deliberately narrow. The exact : no-op had 19/3/10 safe examples and 0 harmful examples on train/dev/test. A broader ip addr show candidate was rejected because a compound dev command introduced one harmful allow.

Scope

This part does not inspect recursive directory reads such as grep -r KEY ., Grep tool requests with glob: ".env", or repository-history reads such as git show HEAD:.env. Those paths behave as they do on main and need separate coverage. The PowerShell tool has a separate path validator, so commands such as Get-Content .env are also outside this PR and continue to behave as they do on main.

SABER rules evaluation

Evaluated all 113,960 records with the default rules:

Split	Harmful allowed before	Harmful allowed after	Safe asked before	Safe asked after
dev	451	151	9,342	9,443
test	435	156	9,337	9,480

The net increase in safe prompts is the expected cost of asking before safe-labeled secret reads. The accepted : relaxation reduces prompts within the hardened result without adding harmful allows.

Testing

• bun test --randomize --seed=1 src/tools/BashTool/bashPermissions.test.ts src/utils/permissions/__tests__/sensitiveReads.test.ts — 26 passed, 0 failed, 130 assertions
• cd desktop && bun run lint — passed
• Windows manual verification passed for Read/Bash prompts, denial, bypass mode, and a real .env file symlink
• full root bun test on the current commit — 2,181 passed, 26 skipped, and 59 environment-dependent Windows/server integration tests failed; neither changed permission test file failed
• desktop Vitest was also attempted; unrelated existing suites failed with signal is not of type AbortSignal and mock initialization errors

Part of #145

[GoDiao]

Thanks @AIR-Xie! The design is right: the check lives in the shared permission layer, so the Read tool and Bash use one definition of a sensitive read, it keeps public keys, known_hosts, config and shell profiles readable as #145 asked, and the exact : no-op is safe (I traced : > f, : $(cmd) and : && rm x; none is allowed by the new regex). CI is green. I traced the points below by reading the code, not by running it, so please confirm them with the tests suggested at the end.

1. Read-only commands without a path extractor still read secrets silently.
tac, rev, fold, expand, unexpand, fmt, pr, comm, cmp and tsort are in the read-only allowlist (readOnlyValidation.ts) but not in PATH_EXTRACTORS, so validateSinglePathCommand returns passthrough for them and they never reach the new check. tac .env and rev ~/.ssh/id_rsa should still run without a prompt. #145's goal is that reads of .env and private keys are asked about, and cat, head and grep are the examples from its data, not the limit. Could you add extractors for these commands (filterOutFlags, operation read, like cat)? The Record<PathCommand, ...> tables should make the compiler point to every place that needs an entry. numfmt takes no file arguments, so it does not need one.

2. Globs are checked by their literal text only.
In the glob branch of validatePath, isSensitiveReadPath sees the pattern, so cat .env* and cat .ssh/id_* are caught, but cat .e*, cat .en?, cat .[e]nv, cat .*, cat ~/.ssh/* and cat ~/.ssh/i* are not, and the shell expands them to the secret. Could you expand a wildcard in the last path segment against its directory (readdirSync plus the glob matcher the repo already uses), check each match with isSensitiveReadPath, and ask if any matches? A wildcard in a directory segment (~/.s*/id_rsa) can be treated as ask. If the directory cannot be read, keep today's behaviour.

3. Bash does not check the original name of a symlink.
For a non-glob path, validatePath passes [resolvedPath] to isPathAllowed, so only the target name is checked. With .env -> envs/prod.conf, cat .env is still allowed, while the Read tool asks, because getPathsForPermissionCheck includes the original path and every hop. The PR description says aliases cannot bypass, which holds for Read only. Could you also call checkSensitiveReadPermission(absolutePath) for read operations once the path is otherwise allowed (default getPathsForPermissionCheck, as in Read)? I would not pass [absolutePath, resolvedPath] into isPathAllowed, since pathInAllowedWorkingPath would then require both to be inside the working directory.

4. Please do not ask in bypass mode.
The ask is a safetyCheck, which permissions.ts (step 1g) returns before the bypass check, so in bypassPermissions every read of .env prompts, and the user's own allow rules are ignored too. The desktop confirmation for bypass says the agent gets unrestricted access, and #146 plans an opt-in safety net for that mode. So the new check should not run when bypass is active, that is mode === 'bypassPermissions', or plan with isBypassPermissionsModeAvailable (the condition at permissions.ts L1268-1271). Could you pass the permission context to checkSensitiveReadPermission, return null when bypass is active, and put that condition in one small helper that both permissions.ts and the new check use? Please keep classifierApprovable: false for the other modes. The user's allow rules (for example Read(.env)) staying ignored is fine for now; we can revisit it later.

5. .env.example and similar template files should not ask.
fileName.startsWith('.env') also matches .env.example, .env.sample, .env.template, .envrc and .environment. Could you match only .env itself and .env.<suffix> where the suffix is not example, sample, template or dist? Then .envrc and .environment stop matching without a special case.

6. Smaller points.

• The new ask has no suggestions, but for Bash pathValidation.ts offers a "always allow reads in this directory" rule that has no effect on .env, so the prompt comes back next time. Could you leave that suggestion out when the reason is the sensitive-read check?
• The PR says Closes #145, but the second goal of the issue (fewer prompts for safe calls) is covered only by :. Please use Part of #145, unless you plan to include more shapes here.
• Recursive reads (grep -r KEY ., the Grep tool with glob: ".env") and git show HEAD:.env are not covered. That is the same as on main and fine for this PR, but please say so in the description and the CHANGELOG so nobody assumes they are.

Tests (each should fail on main or without the matching change):

• tac .env and rev .ssh/id_rsa ask;
• a symlink .env -> envs/prod.conf in a temp project: Bash and Read both ask;
• cat .e*, cat .[e]nv, cat ~/.ssh/* ask;
• case variants .ENV and .ssh/ID_RSA, and a Windows-style path with backslashes;
• .env.example, .env.sample and .envrc do not ask, .env.local does;
• bypass mode and plan started from bypass: no ask; default mode: ask;
• : && rm x is not allowed (the : case only covers : and : $(whoami) today).

Please also run bun test --randomize --seed=1 on the files you touch. Thanks again!

[GoDiao]

Thanks for working through the whole list. Every item from the last review is in, and I checked them against the code: the extractors for the ten read-only commands, glob expansion, the symlink name in Bash, the shared isBypassPermissionsActive helper (also used in permissions.ts), the template exemptions, the suggestion filter, the scope notes and Part of #145. The two test files pass on their own and in a randomized run, and breaking each of those pieces makes a test fail.

Two more changes before we merge. Both make ordinary reads ask, which is the cost #145 asks us to keep low.

1. A glob in a directory segment always asks.
In checkSensitiveGlobReadPermission (src/utils/permissions/pathValidation.ts), when the directory part of the path contains glob characters, the check asks without looking at what the last segment can match. So grep foo packages/*/package.json and cat src/**/*.ts ask on this branch and run on main, and they ask even with a Bash(grep:*) or Bash(cat:*) allow rule, because the path check runs before the allow rules. A literal path such as Next.js's app/[id]/page.tsx is treated as a glob and asks too.

Could you ask only when the last segment's pattern can match a sensitive name (.env, .env.* other than the exempt suffixes, the names in SENSITIVE_READ_FILES, or id_* under .ssh)? Then .s*/id_rsa and */.env still ask, and packages/*/package.json does not. Please add tests for both sides, including app/[id]/page.tsx.

2. Directories named token or credentials ask.
isSensitiveReadPath compares the last path segment with SENSITIVE_READ_FILES without checking whether it is a file. So ls src/token and grep -r foo src/credentials ask, while ls is allowed without a prompt on main. Directories with these names are common in auth code. Could you skip the check when the path is an existing directory, and add a test for ls src/token?

This one was already in the first version, and we missed it in the first review. Sorry for raising it only now.

One note, not a change request: the PowerShell tool has its own path check (src/tools/PowerShellTool/pathValidation.ts) and does not use the new one, so Get-Content .env does not ask. That is outside this PR. Could you add a sentence about it to the Scope section of the description?

[AIR-Xie]

Addressed the latest review in c59177b.

• Directory-segment globs now prompt only when the final segment targets a sensitive name. .s*/id_rsa and */.env still ask; packages/*/package.json, src/**/*.ts, and app/[id]/page.tsx pass through.
• Existing directories named token or credentials no longer trigger the sensitive-file prompt. Sensitive symlink names remain protected.
• The Scope section now notes that PowerShell uses a separate validator and is outside this PR.

Verification:

• Targeted tests: 26 passed, 0 failed, 130 assertions.
• Randomized targeted run with --seed=1: 26 passed, 0 failed.
• Windows manual verification passed for Read/Bash prompts, denial, bypass mode, and a real .env file symlink.
• The full root run and its environment-dependent Windows/server failures are documented in the PR description.

Ready for owner review after CI completes.