jpmschweitzer/clide
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

harden the ConPTY backend (Linux-verifiable pre-VM work)

Browse Source 
Pre-Windows-VM hardening — the parts validatable on Linux, leaving the
unrunnable FFI (Job Object, T-424) for the VM session:

- Clamp PTY cols/rows to >= 2 in both backends' spawn + resize (new
  pty_size.dart). A 1-column ConPTY makes conhost spin emitting CRLF
  (microsoft/terminal#19922); 0 is invalid on both platforms.
- ci/test.sh: --timeout 60s on the dart-test pty line (matches the flutter
  lines) so a wedged ConPTY reader fails fast instead of hanging the run.
- Make windows_pty.dart's pure helpers public + testable off-Windows:
  quoteArg (MSVCRT quoting), composeEnvironmentBlock, and resolveExecutable
  (now takes an injectable existence probe). New windows_pty_args_test.dart
  + pty_size_test.dart give 15 cross-platform assertions over the trickiest
  Windows logic the on-Windows smoke suite can't reach off-platform.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
jpmschweitzer
2026-06-14 20:54:50 +02:00
co-authored by Claude Opus 4.8
parent 9c288a618b
commit 606d3df98f
6 changed files with 166 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/src/pty/native_pty.dart
+5 -4
View File
@@ -25,6 +25,7 @@ import 'dart:typed_data';
import 'package:ffi/ffi.dart';
import 'errors.dart';
import 'pty_size.dart';
import '../ipc/errno_mapping.dart' show PosixErrno;
import 'ffi/libc.dart' as libc;
import 'pty_session.dart';
@@ -310,8 +311,8 @@ class NativePty implements PtySession {
// ---- Set initial winsize on the master ---------------------------
final ws = calloc<_Winsize>()
..ref.wsRow = rows
..ref.wsCol = columns;
..ref.wsRow = clampPtyDimension(rows)
..ref.wsCol = clampPtyDimension(columns);
_ioctl(masterFd, _kTiocsWinsz, ws);
calloc.free(ws);
@@ -429,8 +430,8 @@ class NativePty implements PtySession {
void resize({required int cols, required int rows}) {
if (_dead) return;
final ws = calloc<_Winsize>()
..ref.wsRow = rows
..ref.wsCol = cols;
..ref.wsRow = clampPtyDimension(rows)
..ref.wsCol = clampPtyDimension(cols);
final rc = _ioctl(_fd, _kTiocsWinsz, ws);
calloc.free(ws);
if (rc < 0 && libc.errno == PosixErrno.ebadf) {
lib/src/pty/pty_size.dart
+15
View File
@@ -0,0 +1,15 @@
/// Minimum dimension handed to any PTY backend.
///
/// A 1-column ConPTY makes the Windows conhost spin emitting CRLF forever
/// (microsoft/terminal#19922), and a 0 in either axis is invalid on both
/// platforms. Every backend clamps its spawn + resize through this, so a
/// degenerate size from the UI — a pane measured at zero width during a
/// transient layout pass — can never wedge a child. The floor (2) is below
/// any real terminal, so the clamp is invisible in normal use.
library;
/// Smallest column/row count a PTY backend will accept.
const int minPtyDimension = 2;
/// Clamp a column or row count up to [minPtyDimension].
int clampPtyDimension(int value) => value < minPtyDimension ? minPtyDimension : value;
lib/src/pty/windows_pty.dart
+26 -19
View File
@@ -46,6 +46,7 @@ import 'package:ffi/ffi.dart';
import 'errors.dart';
import 'pty_session.dart';
import 'pty_size.dart';
// -- structs ----------------------------------------------------------------
@@ -248,7 +249,7 @@ class WindowsPty implements PtySession {
String? workingDirectory,
Map<String, String> environment = const {},
}) {
executable = _resolveExecutable(executable, environment);
executable = resolveExecutable(executable, environment);
// ---- Pipes + pseudo console ---------------------------------------
final ha = calloc<_Handle>();
@@ -275,8 +276,8 @@ class WindowsPty implements PtySession {
calloc.free(hb);
final size = calloc<_Coord>()
..ref.x = columns
..ref.y = rows;
..ref.x = clampPtyDimension(columns)
..ref.y = clampPtyDimension(rows);
final hpcOut = calloc<_Handle>();
final hr = _createPseudoConsole(size.ref, inRead, outWrite, 0, hpcOut);
calloc.free(size);
@@ -332,8 +333,8 @@ class WindowsPty implements PtySession {
// parse (which also gives .bat/.cmd their cmd.exe host); the
// executable is pre-resolved to an absolute path above so no PATH
// ambiguity is left at this point.
final cmdLine = [executable, ...arguments].map(_quoteArg).join(' ').toNativeUtf16(allocator: malloc);
final envBlock = _environmentBlock(environment);
final cmdLine = [executable, ...arguments].map(quoteArg).join(' ').toNativeUtf16(allocator: malloc);
final envBlock = composeEnvironmentBlock(environment).toNativeUtf16(allocator: malloc);
final cwdN = workingDirectory == null ? ffi.nullptr : workingDirectory.toNativeUtf16(allocator: malloc);
// STARTF_USESTDHANDLES with NULL std handles (calloc zeroes them):
@@ -508,8 +509,8 @@ class WindowsPty implements PtySession {
void resize({required int cols, required int rows}) {
if (_dead || _hpc == ffi.nullptr) return;
final size = calloc<_Coord>()
..ref.x = cols
..ref.y = rows;
..ref.x = clampPtyDimension(cols)
..ref.y = clampPtyDimension(rows);
_resizePseudoConsole(_hpc, size.ref);
calloc.free(size);
}
@@ -587,7 +588,12 @@ class WindowsPty implements PtySession {
/// Resolve a bare command name against the environment's PATH +
/// PATHEXT (mirrors what the POSIX side does with `:`-split PATH —
/// visible/debuggable resolution instead of CreateProcess magic).
static String _resolveExecutable(String executable, Map<String, String> environment) {
///
/// [exists] overrides the on-disk probe so the resolution logic is
/// unit-testable off-Windows; production passes the default. Public for
/// that reason — not part of the backend's external contract.
static String resolveExecutable(String executable, Map<String, String> environment, {bool Function(String path)? exists}) {
final fileExists = exists ?? ((String path) => File(path).existsSync());
final pathext = (environment['PATHEXT'] ?? Platform.environment['PATHEXT'] ?? '.COM;.EXE;.BAT;.CMD').split(';').where((e) => e.isNotEmpty).toList();
final hasKnownExt = pathext.any((e) => executable.toLowerCase().endsWith(e.toLowerCase()));
@@ -604,7 +610,7 @@ class WindowsPty implements PtySession {
if (executable.contains('\\') || executable.contains('/')) {
for (final c in candidates(executable)) {
if (File(c).existsSync()) return c;
if (fileExists(c)) return c;
}
return executable;
}
@@ -612,14 +618,15 @@ class WindowsPty implements PtySession {
for (final dir in path.split(';')) {
if (dir.isEmpty) continue;
for (final c in candidates('$dir\\$executable')) {
if (File(c).existsSync()) return c;
if (fileExists(c)) return c;
}
}
return executable;
}
/// Quote one argument per MSVCRT command-line parsing rules.
static String _quoteArg(String arg) {
/// Quote one argument per MSVCRT command-line parsing rules. Public so the
/// quoting rules can be unit-tested off-Windows; not an external contract.
static String quoteArg(String arg) {
if (arg.isNotEmpty && !arg.contains(RegExp(r'[ \t"\n\v]'))) return arg;
final b = StringBuffer('"');
var backslashes = 0;
@@ -646,17 +653,17 @@ class WindowsPty implements PtySession {
return b.toString();
}
/// Compose a CREATE_UNICODE_ENVIRONMENT block: `K=V\0...\0\0`,
/// entries sorted case-insensitively by key per CreateProcess docs.
static ffi.Pointer<Utf16> _environmentBlock(Map<String, String> environment) {
/// Compose the body of a CREATE_UNICODE_ENVIRONMENT block: `K=V\0...\0`
/// with one trailing NUL, entries sorted case-insensitively by key per
/// CreateProcess docs. The caller nativizes via `toNativeUtf16`, whose own
/// terminator completes the required double-NUL ending (which also keeps an
/// empty environment block valid). Public for unit testing.
static String composeEnvironmentBlock(Map<String, String> environment) {
final entries = environment.entries.toList()..sort((a, b) => a.key.toUpperCase().compareTo(b.key.toUpperCase()));
// NUL via fromCharCode — an inline NUL escape in a string literal
// is invisible in review and trips up text tooling.
final nul = String.fromCharCode(0);
final joined = entries.map((e) => '${e.key}=${e.value}$nul').join();
// toNativeUtf16 appends the final terminating NUL; the explicit one
// after the last entry completes the required double-NUL ending (and
// keeps an empty environment block valid too).
return '$joined$nul'.toNativeUtf16(allocator: malloc);
return '$joined$nul';
}
}