From 8477e302efd39ad3b9109375a6f4822176477582 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Fri, 12 Jun 2026 00:32:24 +0200 Subject: [PATCH] detect symlinks from the lister entity, never descend them (T-365) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit stat() follows links, so `stat.type == link` was always false: every FileEntry reported isSymlink=false and walkFiles happily descended symlinked directories — an escape hatch out of the workspace and a cycle risk for the search engine built on the walk. The lister already runs with followLinks: false, so the Link entity itself is the signal. listDir keeps reporting the target type for the UI; walkFiles skips descent into symlinked dirs and still emits file symlinks as entries. Co-Authored-By: Claude Fable 5 --- .pql/changelog/ticket_history/2026-06.sql | 2 ++ .pql/changelog/tickets/2026-06.sql | 6 ++++ CHANGELOG.md | 5 +++ lib/src/files/listing.dart | 16 ++++++--- test/files/walk_test.dart | 42 +++++++++++++++++++++++ 5 files changed, 66 insertions(+), 5 deletions(-) diff --git a/.pql/changelog/ticket_history/2026-06.sql b/.pql/changelog/ticket_history/2026-06.sql index fdee59f2..f43bccfe 100644 --- a/.pql/changelog/ticket_history/2026-06.sql +++ b/.pql/changelog/ticket_history/2026-06.sql @@ -3945,3 +3945,5 @@ INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, chang INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHBYTJ4E7ZBY6DWWNT1S16M', 'status', 'in_progress', 'done', NULL, '2026-06-11 22:26:15', '2026-06-11 22:26:15', '2026-06-11 22:26:15', NULL, '05386705854fd48ee75d7a3dfbfd5bc9', 2) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHC0HYRZ86CWW0DDQJ5CAQM', 'status', 'backlog', 'in_progress', NULL, '2026-06-11 22:26:38', '2026-06-11 22:26:38', '2026-06-11 22:26:38', NULL, '6d14a673d6733b5024726bfc227f6711', 2) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHC0HYRZ86CWW0DDQJ5CAQM', 'status', 'in_progress', 'done', NULL, '2026-06-11 22:30:24', '2026-06-11 22:30:24', '2026-06-11 22:30:24', NULL, 'aa1fe975e147be83905f24c63662254d', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHBSG6356MZJ2DCCCSBMBGM', 'status', 'backlog', 'in_progress', NULL, '2026-06-11 22:30:48', '2026-06-11 22:30:48', '2026-06-11 22:30:48', NULL, '2c5c8e5cdf68c96e25705f014dcd6acc', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHBSG6356MZJ2DCCCSBMBGM', 'status', 'in_progress', 'done', NULL, '2026-06-11 22:32:08', '2026-06-11 22:32:08', '2026-06-11 22:32:08', NULL, 'f3dbb31fb6f6f416a2d4d8ef37723316', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-06.sql b/.pql/changelog/tickets/2026-06.sql index a72503b5..fc42bd70 100644 --- a/.pql/changelog/tickets/2026-06.sql +++ b/.pql/changelog/tickets/2026-06.sql @@ -4001,3 +4001,9 @@ INSERT INTO tickets (record_id, type, parent_record_id, title, description, stat Fix: bounds-check the lookahead (ignore incomplete 38/48 sequences), and parse colon-form sub-parameters per ECMA-48/ITU T.416 — treat 38:2::r:g:b and 38;2;r;g;b equivalently. Note T-123 (parser split) touches the same file; coordinate but do not block on it. Acceptance: feeding any truncated/garbled SGR byte sequence never throws (fuzz-style test over partial sequences); colon-form truecolor sets the same fg/bg as semicolon form; existing SGR tests stay green.', 'done', 'high', NULL, NULL, NULL, '2026-06-11 21:56:47', '2026-06-11 22:30:24', NULL, 'b64b55f7d40149848fc687efe1cf8f32', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHBSG6356MZJ2DCCCSBMBGM', 'bug', '06FBHBGHNEQTAEPGNJKN42C1E8', 'listDir symlink detection is dead code; walkFiles descends symlinked dirs', 'lib/src/files/listing.dart:46-54 — stat() follows links, so isSymlink is always false; walkFiles therefore descends symlinked directories the docs claim it skips (escape hatch out of the workspace, plus cycle risk). + +Fix: use lstat (FileStat via Link check / FileSystemEntity.isLinkSync on the raw path) for symlink detection. Tests: symlinked dir is reported as symlink and not descended; symlink cycle does not hang the walk.', 'in_progress', 'high', NULL, NULL, NULL, '2026-06-11 21:55:49', '2026-06-11 22:30:48', NULL, 'aa0f09e575dd2d3232f34485982c1ec5', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHBSG6356MZJ2DCCCSBMBGM', 'bug', '06FBHBGHNEQTAEPGNJKN42C1E8', 'listDir symlink detection is dead code; walkFiles descends symlinked dirs', 'lib/src/files/listing.dart:46-54 — stat() follows links, so isSymlink is always false; walkFiles therefore descends symlinked directories the docs claim it skips (escape hatch out of the workspace, plus cycle risk). + +Fix: use lstat (FileStat via Link check / FileSystemEntity.isLinkSync on the raw path) for symlink detection. Tests: symlinked dir is reported as symlink and not descended; symlink cycle does not hang the walk.', 'done', 'high', NULL, NULL, NULL, '2026-06-11 21:55:49', '2026-06-11 22:32:08', NULL, 'f83e31fd330c98a311a3da10cba1c655', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); diff --git a/CHANGELOG.md b/CHANGELOG.md index c4a14f0a..6acf87e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,11 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. sub-parameters (`38:2:r:g:b`, ITU T.416) parse like the semicolon form instead of being mangled. (T-369) +- **File listing flags symlinks again and the workspace walk no longer + follows them.** Symlink detection was dead code, so `files.walk` (and the + search engine on top of it) silently descended symlinked directories — + including ones pointing outside the workspace. (T-365) + - **Accepting ExitPlanMode now leaves plan mode in the conversation panel.** Approving Claude's plan (the ExitPlanMode tool) transitioned the underlying session out of plan mode, but clide's tracked permission mode didn't follow, diff --git a/lib/src/files/listing.dart b/lib/src/files/listing.dart index 0cc3f9f2..cf5d64f1 100644 --- a/lib/src/files/listing.dart +++ b/lib/src/files/listing.dart @@ -43,6 +43,11 @@ Future> listDir({required Directory root, required String dir, r await for (final e in resolved.list(followLinks: false)) { final name = e.uri.pathSegments.isNotEmpty ? e.uri.pathSegments.where((s) => s.isNotEmpty).last : ''; final rel = dir.isEmpty ? name : '$dir/$name'; + // With followLinks: false the lister yields Link entities for symlinks — + // that's the symlink signal. stat() follows the link (target type/size, + // notFound for broken links), so its type can never be `link` and must + // not be used for detection (T-365). + final isLink = e is Link; final stat = await e.stat(); final isDir = stat.type == FileSystemEntityType.directory; if (ignore.isIgnored(rel, isDirectory: isDir)) continue; @@ -51,7 +56,7 @@ Future> listDir({required Directory root, required String dir, r name: name, path: rel, isDirectory: isDir, - isSymlink: stat.type == FileSystemEntityType.link, + isSymlink: isLink, sizeBytes: isDir ? null : stat.size, modifiedMs: stat.modified.millisecondsSinceEpoch, ), @@ -79,9 +84,10 @@ class WalkResult { /// Recursively walk [root], returning every non-ignored *file* /// (directories are descended into but not emitted), pruned by -/// [ignore]. Reuses [listDir] per directory, so ignore filtering, -/// symlink-escape safety (`followLinks: false`), and per-directory -/// sorting are inherited. +/// [ignore]. Reuses [listDir] per directory, so ignore filtering and +/// per-directory sorting are inherited. Symlinks are never descended — +/// a symlinked directory would be an escape hatch out of the workspace +/// and a cycle risk (T-365); symlinks to files are emitted as entries. /// /// Capped at [maxFiles] to bound work on pathological trees; when the /// cap is hit the walk stops early and [WalkResult.truncated] is set so @@ -97,7 +103,7 @@ Future walkFiles({required Directory root, required IgnoreSet ignore final entries = await listDir(root: root, dir: dir, ignore: ignore); for (final e in entries) { if (e.isDirectory) { - stack.add(e.path); + if (!e.isSymlink) stack.add(e.path); } else { out.add(e); if (out.length >= maxFiles) { diff --git a/test/files/walk_test.dart b/test/files/walk_test.dart index e8a6ea15..5a8c3e90 100644 --- a/test/files/walk_test.dart +++ b/test/files/walk_test.dart @@ -56,4 +56,46 @@ void main() { expect(r.files, isEmpty); expect(r.truncated, isFalse); }); + + // T-365: stat() follows links, so the old detection (stat.type == link) + // was always false and walkFiles descended symlinked directories — + // an escape hatch out of the workspace. + group('symlinks (T-365)', () { + test('listDir reports a symlinked directory as a symlink', () async { + final outside = await Directory.systemTemp.createTemp('clide-walk-outside-'); + addTearDown(() => outside.deleteSync(recursive: true)); + File('${outside.path}/secret.txt').writeAsStringSync('s'); + Link('${root.path}/linked').createSync(outside.path); + + final entries = await listDir(root: root, dir: '', ignore: IgnoreSet([])); + final linked = entries.singleWhere((e) => e.name == 'linked'); + expect(linked.isSymlink, isTrue); + expect(linked.isDirectory, isTrue, reason: 'target type still reported for the UI'); + }); + + test('walkFiles does not descend a symlinked directory', () async { + final outside = await Directory.systemTemp.createTemp('clide-walk-outside-'); + addTearDown(() => outside.deleteSync(recursive: true)); + File('${outside.path}/secret.txt').writeAsStringSync('s'); + Link('${root.path}/linked').createSync(outside.path); + + final r = await walkFiles(root: root, ignore: IgnoreSet([])); + expect(r.files.map((e) => e.path), isNot(contains('linked/secret.txt'))); + }); + + test('a symlink cycle does not hang the walk', () async { + Link('${root.path}/lib/loop').createSync(root.path); + final r = await walkFiles(root: root, ignore: IgnoreSet([])); + expect(r.truncated, isFalse); + expect(r.files.map((e) => e.path), contains('README.md')); + }); + + test('a symlink to a file is emitted as a file entry, flagged', () async { + Link('${root.path}/readme-link').createSync('${root.path}/README.md'); + final r = await walkFiles(root: root, ignore: IgnoreSet([])); + final e = r.files.singleWhere((e) => e.path == 'readme-link'); + expect(e.isSymlink, isTrue); + expect(e.isDirectory, isFalse); + }); + }); }