Fix sed --quiet/--silent classified as 'unknown' (SAFE_SED_OPTION whitelist unreachable)

Open Beginner friendly
#12,215 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
82/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
typescript
Domain
cli, security

Research direction

Start in packages/core/src/utils/shell-safety-rules.ts at classifySedCommandSafety, reading SAFE_SED_OPTION and the second argument loop. Run the provided Node reproduction or equivalent before/after checks for --quiet, --silent, and -n; done means the long aliases classify as read-only while existing cases retain their behavior.

Written by the indexing model from the issue text.

Description

category/core priority/P3 scope/shell status/ready-for-agent type/bug

What happened?

classifySedCommandSafety in packages/core/src/utils/shell-safety-rules.ts lists --quiet and --silent (GNU sed's documented aliases of -n) as safe in the SAFE_SED_OPTION whitelist (lines 16-17), but a guard at line 190 returns 'unknown' for every ---prefixed argument except --line-length, so the whitelist's --quiet/--silent alternatives are unreachable from the branch that consumes it (line 192). The short spelling -n classifies as 'read-only'; the long spellings do not.

This is a read-only false negative: sed --quiet 's/a/b/' file and sed --silent 's/a/b/' file are not classified as read-only, so they are not auto-approved and are not treated as concurrency-safe, while the equivalent sed -n ... runs unprompted. The failure direction is over-conservative only: these commands have no write effect, so this can never silently auto-approve a write. Users get a confirmation prompt for a command that has no write effect.

Reproduction

Self-contained script, driving the shipped v0.24.0 bundle directly (no clone, no build):

// Load the shipped bundle chunk that exports classifySedCommandSafety / isShellCommandReadOnly,
// then call the sed classifier with the long and short spellings of -n.
const fs = require('fs');
const dir = '/root/.local/lib/qwen-code/lib/chunks';
const file = fs.readdirSync(dir).find((n) =>
  fs.readFileSync(dir + '/' + n, 'utf8').includes('classifySedCommandSafety'));
const m = require(dir + '/' + file);
const show = (label, v) => console.log(label.padEnd(46), '=>', v);
show("classifySedCommandSafety(['--quiet','s/a/b/','file'])",
  m.classifySedCommandSafety(['--quiet', 's/a/b/', 'file']));
show("classifySedCommandSafety(['--silent','s/a/b/','file'])",
  m.classifySedCommandSafety(['--silent', 's/a/b/', 'file']));
show("classifySedCommandSafety(['-n','s/a/b/','file'])",
  m.classifySedCommandSafety(['-n', 's/a/b/', 'file']));
show("classifySedCommandSafety(['--line-length=80','l','file'])",
  m.classifySedCommandSafety(['--line-length=80', 'l', 'file']));
show("isShellCommandReadOnly(\"sed --quiet 's/a/b/' file\")",
  m.isShellCommandReadOnly("sed --quiet 's/a/b/' file"));
show("isShellCommandReadOnly(\"sed -n 's/a/b/' file\")",
  m.isShellCommandReadOnly("sed -n 's/a/b/' file"));

Command: node prove.js

Actual behavior
classifySedCommandSafety(['--quiet','s/a/b/','file']) => unknown
classifySedCommandSafety(['--silent','s/a/b/','file']) => unknown
classifySedCommandSafety(['-n','s/a/b/','file']) => read-only
classifySedCommandSafety(['--line-length=80','l','file']) => read-only
isShellCommandReadOnly("sed --quiet 's/a/b/' file") => false
isShellCommandReadOnly("sed -n 's/a/b/' file") => true

EXIT=0

The same classifier output is reproduced against the upstream source at v0.24.0, loaded directly with Node's native TypeScript type-stripping (see the before/after pair under Verification).

What did you expect to happen?

sed --quiet 's/a/b/' file and sed --silent 's/a/b/' file should classify as 'read-only', exactly like sed -n 's/a/b/' file, because GNU sed documents --quiet and --silent as aliases of -n and SAFE_SED_OPTION explicitly lists them. As read-only they would be auto-approved and counted concurrency-safe.

Client information

Headless equivalent of /about; no interactive /about output was available in this environment.

  • qwen-code version: 0.24.0 (read from /root/.local/lib/qwen-code/package.json)
  • Install method: local install rooted at /root/.local/lib/qwen-code, launcher /root/.local/bin/qwen (entry scripts/cli-entry.js); the installed tree is an esbuild bundle (lib/chunks/*.js, about 568 files, no sourcemaps)
  • Node: v24.18.0
  • Platform: Linux FX506LI 6.17.0-35-generic #35~24.04.1-Ubuntu SMP PREEMPT_DYNAMIC Tue May 26 19:30:42 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

Root cause

In packages/core/src/utils/shell-safety-rules.ts at v0.24.0, inside the second for loop of classifySedCommandSafety (function starts at line 145), lines 190-192:

    } else if (/^--(?!line-length(?:=|$))/.test(arg)) {
      return 'unknown';
    } else if (arg.startsWith('-') && !SAFE_SED_OPTION.test(arg)) {
      return 'unknown';

SAFE_SED_OPTION, defined at lines 16-17:

const SAFE_SED_OPTION =
  /^(?:-[nElrsuz]|--(?:quiet|silent|line-length(?:=.*)?))$/;

The line-190 guard is evaluated before the whitelist test on line 192 and matches every ---prefixed argument that is not --line-length or --line-length=..., so it short-circuits to 'unknown' before line 192 is ever consulted for --quiet/--silent. The whitelist's --(?:quiet|silent) alternatives are therefore dead from this consuming branch: line 192 can only ever see single-dash arguments plus the two --line-length forms. The whitelist declares safe options that its only consumer cannot reach.

classifySedCommandSafety is the single source of truth for the AST path (shellAstParser.ts evaluateSedSafety) and, after its own option loop, for the deprecated regex checker (shellReadOnlyChecker.ts), so the false negative propagates to both the read-only auto-approval path (isShellCommandReadOnlyASTInDirectory) and the concurrency-safety path (isToolCallConcurrencySafe -> isShellCommandReadOnly). The consumer is present in the shipped bundle (chunk-SNPYTKXI.js):

function isToolCallConcurrencySafe(name, kind, args) {
  if (canonicalToolName(name) === ToolNames.AGENT) return true;
  if (kind === "execute" /* Execute */) {
    const command = args?.command;
    if (typeof command !== "string") return false;
    try {
      return isShellCommandReadOnly(command);
    } catch {
      return false;
    }
  }

Correction to an earlier framing of this report: the deprecated regex checker does not auto-allow this input either. isShellCommandReadOnly("sed --quiet 's/a/b/' file") returns false as well (it delegates to the same classifier), so the impact is the prompt/concurrency-safety false negative, not a bypass.

Proposed fix

--- a/packages/core/src/utils/shell-safety-rules.ts
+++ b/packages/core/src/utils/shell-safety-rules.ts
@@ -187,8 +187,6 @@ export function classifySedCommandSafety
       if (script.startsWith('-')) return 'unknown';
       scripts.push(script);
       scriptArguments.add(i);
-    } else if (/^--(?!line-length(?:=|$))/.test(arg)) {
-      return 'unknown';
     } else if (arg.startsWith('-') && !SAFE_SED_OPTION.test(arg)) {
       return 'unknown';
     } else if (!arg.startsWith('-') && scripts.length === 0) {

Deleting the guard makes line 192 the sole authority for unconsumed option arguments, which is exactly the whitelist that already lists --quiet/--silent; no other branch changes, so the fix is minimal.

Residual risk (offered as a suggestion, not a pull request): the change was not run through the project's own test suite here (vitest and monorepo dependencies absent), so there is no CI-level confirmation; the equivalence argument rests on a differential sweep against the pristine source. One second observable delta beside the intended one: ['--quiet','w out','file'] changes from 'unknown' to 'write', which is strictly more accurate for sed --quiet 'w out' file (that script does write) and is still not auto-approved, because the consumers only auto-approve 'read-only'; it is not a safety regression. Legitimate GNU sed long aliases of already-allowed short flags (--regexp-extended, --zero-terminated, --posix, --debug, --sandbox, --null-data) remain 'unknown' both before and after. No regression test for --quiet/--silent is added by this diff.

Verification

Before/after on the unmodified upstream source at v0.24.0 (fetched with gh api ...?ref=v0.24.0), loaded directly with Node v24.18.0 native TypeScript type-stripping.

Pristine source, before the fix:

['--quiet','s/a/b/','file']                    => "unknown"
['--silent','s/a/b/','file']                   => "unknown"
['-n','s/a/b/','file']                         => "read-only"
['--line-length=80','l','file']                => "read-only"
['--quiet','-e','s/a/b/','file']               => "unknown"
['-i','--quiet','s/a/b/','file']               => "write"

EXIT=0

Patched source, after applying the diff above with patch -p1 --dry-run then patch -p1 (both clean):

['--quiet','s/a/b/','file']                    => "read-only"
['--silent','s/a/b/','file']                   => "read-only"
['-n','s/a/b/','file']                         => "read-only"
['--line-length=80','l','file']                => "read-only"
['--quiet','-e','s/a/b/','file']               => "unknown"
['-i','--quiet','s/a/b/','file']               => "write"

EXIT=0

Shipped v0.24.0 bundle (chunk-QPTE6BKL.js) also exhibits the defect:

classifySedCommandSafety(['--quiet','s/a/b/','file']) => unknown
classifySedCommandSafety(['--silent','s/a/b/','file']) => unknown
classifySedCommandSafety(['-n','s/a/b/','file']) => read-only
classifySedCommandSafety(['--line-length=80','l','file']) => read-only

EXIT=0

Differential regression sweep, pristine vs patched, 42 argument vectors:

DIFF ["--quiet","s/a/b/","file"] before=unknown after=read-only
DIFF ["--silent","s/a/b/","file"] before=unknown after=read-only
total cases = 42  divergences = 2

A second sweep of 13 vectors:

same  ["--regexp-extended","s/a/b/","file"]          before=unknown after=unknown
same  ["--zero-terminated","s/a/b/","file"]          before=unknown after=unknown
same  ["--posix","s/a/b/","file"]                    before=unknown after=unknown
same  ["--debug","s/a/b/","file"]                    before=unknown after=unknown
same  ["--sandbox","s/a/b/","file"]                  before=unknown after=unknown
same  ["--null-data","s/a/b/","file"]                before=unknown after=unknown
DIFF  ["--quiet","--silent","s/a/b/","file"]         before=unknown after=read-only
same  ["--quiet","-i","s/a/b/","file"]               before=write after=write
DIFF  ["--quiet","w out","file"]                     before=unknown after=write
same  ["--silent","s/a/b/e","file"]                  before=unknown after=unknown
same  ["--quiet","file"]                             before=unknown after=unknown
DIFF  ["--silent"]                                   before=unknown after=read-only
DIFF  ["--quiet","--","s/a/b/","file"]               before=unknown after=read-only
total = 13  divergences = 4

GNU sed documents the alias: sed --help prints, on line 3, -n, --quiet, --silent (cancel auto-print).

An independent code review re-checked the fix from scratch and confirmed it. It fetched v0.24.0, applied the diff in a scratch tree, reproduced the before/after pair (['--quiet',...] "unknown" -> "read-only", same for --silent), confirmed the shipped v0.24.0 bundle exhibits the defect, and ran its own 22,550-vector differential sweep: 1202 divergences, histogram {unknown->read-only: 1074, unknown->write: 128}, 0 read-only->non-read-only, 0 other->read-only. Verdict: the patch is a correct, minimal, complete fix.

Upstream main is byte-identical to v0.24.0 for this file (diff of the two fetched copies produced no output), so the defect is live on main too. Issue/PR search (open and closed, several phrasings) found no prior report of it; the only hit on the identifiers is the merged refactor PR #7053 that introduced this code.

Dominant language
TypeScript
Stars
28k
Forks
3.1k
Avg merge
1d 1h
Merged PRs (30d)
705

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from QwenLM/qwen-code

All issues in QwenLM/qwen-code

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.