Stop misclassifying innocuous shell redirections (1.4.0)

CHANGELOG.md

@@ -1,3 +1,34 @@
+## 1.4.0
+
+### Added
+
+- **File-descriptor redirections are modelled.** New `RedirectionType` cases
+  `mergeStreams` (`2>&1`, `1>&2`, `>&2`, `>&-`), `combinedOutput` (`&>file`,
+  `>&file`) and `combinedAppendOutput` (`&>>file`). All three shell parsers
+  (bash/POSIX, Windows `cmd`, PowerShell) now recognise these forms.
+- **Benign-redirection marker.** A new `BenignRedirectionDetector` (code
+  `benign-redirection`, in the default suite) records stream merges and
+  null-sink discards as explicit `SecurityLevel.safe` findings for audit
+  visibility. Being below every actionable level, it never changes a decision.
+
+### Changed
+
+- Innocuous redirections no longer over-report a filesystem write. Stream
+  merges (`2>&1`) touch no file, and discards to a null sink (`/dev/null`,
+  `NUL`, `$null`) write/read no real file, so none of them contribute
+  `writeFilesystem`/`readFilesystem` (and thus no `modifyFiles` effect). A
+  genuine target such as `> out.txt` or `&> out.log` still does.
+
+### Fixed
+
+- **`2>&1` is no longer mis-parsed.** Previously the bash tokenizer only knew
+  `2>`/`2>>`, so `cmd 2>&1` produced a target-less `2>` redirection, a spurious
+  "Redirection without a target" diagnostic, and a phantom `CommandInvocation`
+  with executable `1`. `1>&2` and `&>/dev/null` broke the same way. These now
+  parse as a single clean command with the correct redirection and no
+  diagnostic. The discard-everything idiom `cmd >/dev/null 2>&1` parses as one
+  command with two redirections.
+
 ## 1.3.0
 
 ### Added

lib/command_shield.dart

@@ -73,6 +73,7 @@ export 'src/policies/length_limit_policy.dart';
 export 'src/policies/path_traversal_policy.dart';
 export 'src/policies/risk_threshold_policy.dart';
 export 'src/policies/shell_execution_policy.dart';
+export 'src/security/detectors/benign_redirection_detector.dart';
 export 'src/security/detectors/command_substitution_detector.dart';
 export 'src/security/detectors/dangerous_operator_detector.dart';
 export 'src/security/detectors/destructive_command_detector.dart';

lib/src/ast/command_node.dart

@@ -31,6 +31,24 @@ enum RedirectionType {
 
   /// `2>>` — append standard error to a file.
   appendErrorOutput,
+
+  /// File-descriptor duplication that merges one stream into another, e.g.
+  /// `2>&1` (stderr → stdout), `1>&2` (stdout → stderr), `>&2`, or `>&-`
+  /// (close fd). No file is written, so this is an innocuous no-op for the
+  /// filesystem. [RedirectionNode.target] holds the redirection as written
+  /// (e.g. `2>&1`).
+  ///
+  /// Only single-digit file descriptors are modelled; multi-digit fds such as
+  /// `12>&3` are not recognised as merges.
+  mergeStreams,
+
+  /// `&>file` / `>&file` — truncate-and-write both stdout and stderr to a
+  /// file. Writes the file just like [output].
+  combinedOutput,
+
+  /// `&>>file` — append both stdout and stderr to a file. Writes the file
+  /// just like [appendOutput].
+  combinedAppendOutput,
 }
 
 /// The root of the typed command abstract-syntax tree (AST).

lib/src/capabilities/capability_detector.dart

@@ -57,10 +57,29 @@ final class CapabilityDetector {
       case RedirectionType.appendOutput:
       case RedirectionType.errorOutput:
       case RedirectionType.appendErrorOutput:
-        out.add(CommandCapability.writeFilesystem);
+      case RedirectionType.combinedOutput:
+      case RedirectionType.combinedAppendOutput:
+        // Discarding a stream to a null sink (`>/dev/null`, `2>NUL`) writes no
+        // real file, so it grants no filesystem-write capability.
+        if (!_isNullSink(redir.target)) {
+          out.add(CommandCapability.writeFilesystem);
+        }
       case RedirectionType.input:
       case RedirectionType.hereDocument:
-        out.add(CommandCapability.readFilesystem);
+        if (!_isNullSink(redir.target)) {
+          out.add(CommandCapability.readFilesystem);
+        }
+      case RedirectionType.mergeStreams:
+        // Fd duplication (`2>&1`, `1>&2`) merges streams without touching the
+        // filesystem — an innocuous no-op for capability purposes.
+        break;
     }
   }
+
+  /// Whether [target] is a discard sink that reads/writes no real file:
+  /// `/dev/null` (POSIX), `NUL`/`NUL:` (Windows), or `$null` (PowerShell).
+  static bool _isNullSink(String target) {
+    final t = target.toLowerCase();
+    return t == '/dev/null' || t == 'nul' || t == 'nul:' || t == r'$null';
+  }
 }

lib/src/parser/powershell_parser.dart

@@ -47,6 +47,7 @@ enum _PsTokenType {
   newline,
   redirOut,
   redirAppend,
+  redirMerge, // fd duplication: 2>&1, 1>&2, >&1
 }
 
 class _PsToken {
@@ -67,9 +68,11 @@ class _PsToken {
   bool get isRedirection =>
       type == _PsTokenType.redirOut || type == _PsTokenType.redirAppend;
 
-  RedirectionType get redirectionType => type == _PsTokenType.redirAppend
-      ? RedirectionType.appendOutput
-      : RedirectionType.output;
+  RedirectionType get redirectionType => switch (type) {
+    _PsTokenType.redirAppend => RedirectionType.appendOutput,
+    _PsTokenType.redirMerge => RedirectionType.mergeStreams,
+    _ => RedirectionType.output,
+  };
 }
 
 class _PsTokenizer {
@@ -121,14 +124,35 @@ class _PsTokenizer {
           _pos++;
           continue;
         case '>':
-          if (_peek(1) == '>') {
+          if (_peek(1) == '&') {
+            final gtStart = _pos;
+            _pos++; // move onto the `&`
+            final dup = _tryReadFdDup(gtStart, '>');
+            if (dup != null) {
+              tokens.add(dup);
+            } else {
+              _pos++; // consume `&`; the filename follows as a word target.
+              tokens.add(_PsToken(_PsTokenType.redirOut, '>&', gtStart));
+            }
+          } else if (_peek(1) == '>') {
             tokens.add(_PsToken(_PsTokenType.redirAppend, '>>', _pos));
             _pos += 2;
           } else {
             tokens.add(_PsToken(_PsTokenType.redirOut, '>', _pos));
             _pos++;
           }
           continue;
+        case '0':
+        case '1':
+        case '2':
+          // A leading single-digit fd is a redirection only when immediately
+          // followed by `>` (e.g. `2>&1`); otherwise it is a normal word.
+          if (_peek(1) == '>') {
+            tokens.add(_readFdRedirect());
+            continue;
+          }
+          tokens.add(_scanWord());
+          continue;
         default:
           tokens.add(_scanWord());
       }
@@ -149,6 +173,58 @@ class _PsTokenizer {
   static final RegExp _nameStart = RegExp(r'[A-Za-z_]');
   static final RegExp _nameChar = RegExp(r'[A-Za-z0-9_]');
 
+  static bool _isDigit(String? ch) =>
+      ch != null && ch.codeUnitAt(0) >= 0x30 && ch.codeUnitAt(0) <= 0x39;
+
+  static bool _isFdBoundary(String? ch) => ch == null || _isTerminator(ch);
+
+  /// Parses a redirection that begins with an explicit fd digit, with `_pos`
+  /// on the digit and `_peek(1) == '>'`. Handles fd duplication (`2>&1`),
+  /// append (`2>>`) and plain (`2>`) forms. The non-merge forms collapse to
+  /// output redirection.
+  _PsToken _readFdRedirect() {
+    final start = _pos;
+    final fd = input[_pos];
+    if (_peek(2) == '&') {
+      _pos += 2; // move onto the `&`
+      final dup = _tryReadFdDup(start, '$fd>');
+      if (dup != null) return dup;
+      _pos++; // consume `&`; the filename follows as a word target.
+      return _PsToken(_PsTokenType.redirOut, '$fd>&', start);
+    }
+    if (_peek(2) == '>') {
+      _pos += 3;
+      return _PsToken(_PsTokenType.redirAppend, '$fd>>', start);
+    }
+    _pos += 2;
+    return _PsToken(_PsTokenType.redirOut, '$fd>', start);
+  }
+
+  /// With `_pos` on the `&` of a `…>&…` redirection, returns a
+  /// [_PsTokenType.redirMerge] token when a valid fd reference (digits or `-`)
+  /// terminated by a word boundary follows, consuming it. Returns `null`
+  /// otherwise, leaving `_pos` on the `&`.
+  _PsToken? _tryReadFdDup(int start, String prefix) {
+    final after = _peek(1);
+    if (after == '-') {
+      if (_isFdBoundary(_peek(2))) {
+        _pos += 2; // consume `&-`
+        return _PsToken(_PsTokenType.redirMerge, '$prefix&-', start);
+      }
+      return null;
+    }
+    if (!_isDigit(after)) return null;
+    var ahead = 1;
+    final digits = StringBuffer();
+    while (_isDigit(_peek(ahead))) {
+      digits.write(_peek(ahead));
+      ahead++;
+    }
+    if (!_isFdBoundary(_peek(ahead))) return null;
+    _pos += ahead; // consume `&` plus the fd digits
+    return _PsToken(_PsTokenType.redirMerge, '$prefix&$digits', start);
+  }
+
   _PsToken _scanWord() {
     final start = _pos;
     final buffer = StringBuffer();
@@ -405,6 +481,17 @@ class _PsTokenParser {
         _i++;
         continue;
       }
+      if (tok.type == _PsTokenType.redirMerge) {
+        // Fd duplication (`2>&1`) carries its own target and consumes no word.
+        redirections.add(
+          RedirectionNode(
+            type: RedirectionType.mergeStreams,
+            target: tok.value,
+          ),
+        );
+        _i++;
+        continue;
+      }
       if (tok.isRedirection) {
         _i++;
         if (!_atEnd && tokens[_i].type == _PsTokenType.word) {