confine editor.open/editor.save to the workspace (T-363)

The editor registry resolved buffer paths with a string join that
passed absolute paths through verbatim and never normalized `..` —
an unconfined read and write primitive over IPC while files.read was
carefully guarded. Buffer paths now resolve through
resolveUnderRootFollowingSymlinks: traversal, absolute escapes, and
symlinks-out are rejected at open, and re-checked at save so a
symlink swapped in under an open buffer's path can't redirect the
write. D-80's extra read roots deliberately do not apply — a buffer
is a write surface. Handlers map PathOutsideRoot to the same error
files.read uses. Also merges a duplicate Added heading that had crept
into the Unreleased changelog section.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
2026-06-12 00:39:23 +02:00
co-authored by Claude Fable 5
parent 88d72789f4
commit ba6ab51118
6 changed files with 87 additions and 9 deletions
@@ -3949,3 +3949,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 ('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;
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 ('06FBHBR4636GSRJBWFJDAZ6ZA0', 'status', 'backlog', 'in_progress', NULL, '2026-06-11 22:32:29', '2026-06-11 22:32:29', '2026-06-11 22:32:29', NULL, '7b833a606da59ab523e0bb43f1753f6a', 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 ('06FBHBR4636GSRJBWFJDAZ6ZA0', 'status', 'in_progress', 'done', NULL, '2026-06-11 22:34:31', '2026-06-11 22:34:31', '2026-06-11 22:34:31', NULL, '254ae09f42b4da439a04d59d675d8f42', 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 ('06FBHBPQE4J4YBJX92812ZK6DR', 'status', 'backlog', 'in_progress', NULL, '2026-06-11 22:34:51', '2026-06-11 22:34:51', '2026-06-11 22:34:51', NULL, '14a7d3d6d6c71cae423ce12c71b6c540', 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 ('06FBHBPQE4J4YBJX92812ZK6DR', 'status', 'in_progress', 'done', NULL, '2026-06-11 22:39:04', '2026-06-11 22:39:04', '2026-06-11 22:39:04', NULL, 'ad1c483fe26d5e417b2e1cd8986114f3', 2) ON CONFLICT(hash) DO NOTHING;
+6
View File
@@ -4013,3 +4013,9 @@ Fix: apply the same glob filtering the search side uses before rewriting. Test:
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 ('06FBHBR4636GSRJBWFJDAZ6ZA0', 'bug', '06FBHBGHNEQTAEPGNJKN42C1E8', 'search.replace silently ignores its include/exclude globs', 'lib/src/search/replace_engine.dart:124-143 — the include/exclude glob filters the user typed are accepted but never applied; replace will happily rewrite files outside the filter.
Fix: apply the same glob filtering the search side uses before rewriting. Test: replace with an include glob touches only matching files; exclude glob is honored.', 'done', 'high', NULL, NULL, NULL, '2026-06-11 21:55:38', '2026-06-11 22:34:31', NULL, 'e86c024d6f8a6d9cd9b7273522b54511', 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 ('06FBHBPQE4J4YBJX92812ZK6DR', 'bug', '06FBHBGHNEQTAEPGNJKN42C1E8', 'editor.open/editor.save skip path confinement entirely', 'lib/src/editor/registry.dart:215-219 returns absolute paths verbatim — no .. normalization, no path_safety call — an unconfined read AND write primitive over IPC while files.read is carefully guarded.
Fix: route editor.open/editor.save through path_safety like files.*. Tests: traversal and absolute-escape attempts rejected for both verbs. Longer-term the confinement should move to the dispatch layer (see the systemic ticket filed with this epic).', 'in_progress', 'high', NULL, NULL, NULL, '2026-06-11 21:55:26', '2026-06-11 22:34:51', NULL, 'ff2c4e85e11c02c88eff6f8e87348d34', 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 ('06FBHBPQE4J4YBJX92812ZK6DR', 'bug', '06FBHBGHNEQTAEPGNJKN42C1E8', 'editor.open/editor.save skip path confinement entirely', 'lib/src/editor/registry.dart:215-219 returns absolute paths verbatim — no .. normalization, no path_safety call — an unconfined read AND write primitive over IPC while files.read is carefully guarded.
Fix: route editor.open/editor.save through path_safety like files.*. Tests: traversal and absolute-escape attempts rejected for both verbs. Longer-term the confinement should move to the dispatch layer (see the systemic ticket filed with this epic).', 'done', 'high', NULL, NULL, NULL, '2026-06-11 21:55:26', '2026-06-11 22:39:04', NULL, '50c42cb7af7715e9ea6417212be0ebdc', 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);
+11 -6
View File
@@ -23,6 +23,12 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit.
below the result — connected only while the card is expanded. Commands with no
followable file show a muted "nothing to follow" note. (T-325)
- **Double-tap-modifier shortcuts (e.g. double-Shift "Search Everywhere").**
The keymap can now bind a bare modifier and a double-tap sequence
(`shift shift`). All four presets (default, vim, vscode, jetbrains) map
double-Shift to the quick-open finder — JetBrains' "Search Everywhere"
gesture, aliased to clide's existing fuzzy file finder. (T-341)
### Changed
- **Each spawned subagent gets its own collapsing activity card.** A fan-out of
@@ -68,13 +74,12 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit.
so the mode indicator and composer stayed stuck on "plan". The approval now
syncs the tracked mode to `default`. (T-337)
### Added
### Security
- **Double-tap-modifier shortcuts (e.g. double-Shift "Search Everywhere").**
The keymap can now bind a bare modifier and a double-tap sequence
(`shift shift`). All four presets (default, vim, vscode, jetbrains) map
double-Shift to the quick-open finder — JetBrains' "Search Everywhere"
gesture, aliased to clide's existing fuzzy file finder. (T-341)
- **`editor.open` / `editor.save` are now workspace-confined.** Both verbs
accepted absolute paths and `..` traversal verbatim — an unconfined read
and write primitive over IPC. They now pass the same path-safety guard as
`files.read`, including a symlink re-check at save time. (T-363)
## [2.3.3] — 2026-06-11
+19 -1
View File
@@ -13,6 +13,7 @@ import 'dart:io' show FileSystemException;
import '../editor/buffer.dart' show Selection;
import '../editor/registry.dart';
import '../files/path_safety.dart' show PathOutsideRoot;
import '../ipc/command_schema.dart';
import '../ipc/envelope.dart';
import '../ipc/errno_mapping.dart';
@@ -84,6 +85,13 @@ Future<IpcResponse> _open(IpcRequest req, EditorRegistry r) async {
r.setSelection(buf.id, Selection.collapsed(_offsetForLine(buf.content, line)));
}
return IpcResponse.ok(id: req.id, data: buf.toJson());
} on PathOutsideRoot {
// Same containment contract as files.read (T-363); a buffer is a
// write surface, so no D-80 extra-root widening here.
return IpcResponse.err(
id: req.id,
error: IpcError(code: IpcExitCode.toolError, kind: IpcErrorKind.toolError, message: 'path outside workspace: $path'),
);
} on FileSystemException catch (e) {
final errno = e.osError?.errorCode;
if (errno != null) {
@@ -190,7 +198,17 @@ Future<IpcResponse> _setContent(IpcRequest req, EditorRegistry r) async {
Future<IpcResponse> _save(IpcRequest req, EditorRegistry r) async {
final id = _resolveId(req, r);
if (id == null) return _notFound(req.id, 'no active buffer');
final ok = await r.save(id);
final bool ok;
try {
ok = await r.save(id);
} on PathOutsideRoot {
// Defense in depth — open already validates, but a symlink can be
// swapped in under the buffer's path between open and save (T-363).
return IpcResponse.err(
id: req.id,
error: IpcError(code: IpcExitCode.toolError, kind: IpcErrorKind.toolError, message: 'path outside workspace'),
);
}
if (!ok) return _notFound(req.id, 'no such buffer: $id');
return IpcResponse.ok(id: req.id, data: {'id': id, 'saved': true});
}
+6 -2
View File
@@ -9,6 +9,7 @@ library;
import 'dart:convert';
import 'dart:io';
import '../files/path_safety.dart';
import '../ipc/envelope.dart';
import '../panes/event_sink.dart';
import 'buffer.dart';
@@ -212,10 +213,13 @@ class EditorRegistry {
events.emit(IpcEvent(subsystem: 'editor', kind: kind, timestamp: DateTime.now().toUtc(), data: data));
}
/// Resolve a buffer path to disk under the workspace root, with the
/// same traversal/symlink containment as files.* (T-363). A buffer is
/// a WRITE surface (save), so the D-80 extra read roots do not apply —
/// strictly workspace-confined. Throws [PathOutsideRoot] on escape.
String _absolutePathOf(String repoRelative) {
if (repoRelative.startsWith('/')) return repoRelative;
final sep = Platform.pathSeparator;
return '${workspaceRoot.absolute.path}$sep${repoRelative.replaceAll('/', sep)}';
return resolveUnderRootFollowingSymlinks(workspaceRoot, repoRelative.replaceAll('/', sep));
}
// Support JSON decode of Selection from IPC args.
+43
View File
@@ -2,6 +2,7 @@ import 'dart:io';
import 'package:clide/clide.dart';
import 'package:clide/src/editor/registry.dart';
import 'package:clide/src/files/path_safety.dart' show PathOutsideRoot;
import 'package:test/test.dart';
void main() {
@@ -212,4 +213,46 @@ void main() {
expect(changed.map((e) => e.data['id']), contains(readme.id));
});
});
// T-363: editor.open/save returned absolute paths verbatim and did no
// `..` normalization — an unconfined read AND write primitive over IPC
// while files.read was carefully guarded.
group('path confinement (T-363)', () {
test('open rejects .. traversal out of the workspace', () async {
final outside = await Directory.systemTemp.createTemp('clide-editor-outside-');
addTearDown(() => outside.deleteSync(recursive: true));
await File('${outside.path}/secret.txt').writeAsString('secret');
final escape = '../${outside.path.split('/').last}/secret.txt';
await expectLater(reg.open(escape), throwsA(isA<PathOutsideRoot>()));
});
test('open rejects absolute paths outside the workspace', () async {
await expectLater(reg.open('/etc/hostname'), throwsA(isA<PathOutsideRoot>()));
});
test('open accepts an absolute path inside the workspace', () async {
final buf = await reg.open('${sandbox.path}/README.md');
expect(buf.content, contains('Hello'));
});
test('open rejects a symlink pointing outside the workspace', () async {
final outside = await Directory.systemTemp.createTemp('clide-editor-outside-');
addTearDown(() => outside.deleteSync(recursive: true));
await File('${outside.path}/secret.txt').writeAsString('secret');
Link('${sandbox.path}/sneaky').createSync('${outside.path}/secret.txt');
await expectLater(reg.open('sneaky'), throwsA(isA<PathOutsideRoot>()));
});
test('save rejects a buffer whose path now symlinks outside', () async {
// Open a legitimate file, then swap a symlink in under its path.
final buf = await reg.open('victim.txt');
reg.setContent(buf.id, 'attacker-controlled');
final outside = await Directory.systemTemp.createTemp('clide-editor-outside-');
addTearDown(() => outside.deleteSync(recursive: true));
await File('${outside.path}/target.txt').writeAsString('original');
Link('${sandbox.path}/victim.txt').createSync('${outside.path}/target.txt');
await expectLater(reg.save(buf.id), throwsA(isA<PathOutsideRoot>()));
expect(await File('${outside.path}/target.txt').readAsString(), 'original');
});
});
}