-
Notifications
You must be signed in to change notification settings - Fork 3k
Harden auto mode self-modification checks #4572
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
2faa4d3
0ed3ce5
c6aabe2
31016a4
1cae556
7d148a9
a964d13
59079fd
7ec50ee
7ed81ad
67afded
6748944
43ea551
d23569a
ff7e40f
d24dee1
7c49a91
be9456c
db36135
2a146c7
c5cbd7d
05d5ff4
b9d3906
25dfdff
ab3e7c3
e66a4c9
050456f
19c5f7a
8d3e1ae
cd34b44
59d2496
86b73a4
633a108
4d4174d
586b7cf
f2cf68b
980a545
268e174
0bf65a1
3d5c7cd
16dfa83
aa5b9b6
2b33bbe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,6 +67,7 @@ import { | |
| recordAllow, | ||
| recordFallbackApprove, | ||
| shouldFallback, | ||
| shouldForceAutoModeReviewForAllow, | ||
| shouldRunAutoModeForCall, | ||
| } from '@qwen-code/qwen-code-core'; | ||
| import { getCommandSubcommandNames } from '../../services/commandMetadata.js'; | ||
|
|
@@ -1930,14 +1931,17 @@ export class Session implements SessionContext { | |
| } | ||
|
|
||
| // Explicit allow (user rule matched, or tool's L3 default is 'allow') | ||
| // is authoritative — AUTO classifier must not be allowed to override | ||
| // it. Parallels coreToolScheduler.ts:1337-1366; without this, an ACP | ||
| // session in AUTO mode could see a user-written `Bash(git push *)` | ||
| // allow rule reach the classifier and get blocked by a conservative | ||
| // Stage-1 verdict. Also resets the denialTracking streak so a | ||
| // following classifier-eligible call doesn't surprise the user with | ||
| // a manual prompt right after an allow-rule call just worked. | ||
| let autoModeAllowed = finalPermission === 'allow'; | ||
| // is authoritative for ordinary calls. In AUTO, protected | ||
| // self-modification writes must still reach the classifier/fail-closed | ||
| // path so allow rules cannot bypass AUTO mode's safety boundary. | ||
| // Also resets the denialTracking streak so a following | ||
| // classifier-eligible call doesn't surprise the user with a manual | ||
| // prompt right after an allow-rule call just worked. | ||
| const forceAutoReviewForAllow = | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] The ACP Session integration adds The coreToolScheduler has one test (approved path only); the ACP path has none. If the ACP path has a logic error (e.g., Suggested fix: Add at least one test in — qwen3.7-max via Qwen Code /review |
||
| approvalMode === ApprovalMode.AUTO && | ||
| shouldForceAutoModeReviewForAllow(pmCtx); | ||
| let autoModeAllowed = | ||
| finalPermission === 'allow' && !forceAutoReviewForAllow; | ||
| if (autoModeAllowed && approvalMode === ApprovalMode.AUTO) { | ||
| this.config.setAutoModeDenialState( | ||
| recordAllow(this.config.getAutoModeDenialState()), | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] Docs/code prefix mismatch: this line still reads
Permission for this action has been denied. Reason: <reason>, but commit3d5c7cd84changedautoMode.ts:602to emitBlocked by auto mode policy: <reason>. The PR diff actually reverted this doc line from the correct new prefix back to the old one.— qwen3.7-max via Qwen Code /review