Skip to content

Commit cd01bc5

Browse files
elkaixelkaix
committed
fix: restore workspace-trust gating for additional dirs
The rollback over-deleted: main gates local.toml additional directories behind workspace trust and rejects broad-scope entries, but ff558ad removed the gate, letting a repo-supplied local.toml expand workspace scope silently (additional_dir = ["/"] loads the whole filesystem on an untrusted workspace), and switched the non-persisting addDir path to the validating loader so a stale or malformed local.toml blocked valid ephemeral additions. Restore main's design on top of the current trust service: locateAdditionalDirsConfig for location-only lookups, trust-gated reload and persistence, broad-scope rejection, and the Program wiring that passes the trust service into WorkspaceDirsService. Also drop the awaited work promise in runGit's timeout path so a surviving pipe owner can no longer hang cleanup, and give the constructor's watch-setup chain a rejection handler. Tests: restored the trust-gating suite and added pins for the filesystem-root claim (trusted and untrusted) and for ephemeral adds surviving rejected ready.
1 parent 660d626 commit cd01bc5

6 files changed

Lines changed: 381 additions & 18 deletions

File tree

‎packages/agent-core-v2/src/app/projectLocalConfig/projectLocalConfig.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
import { createDecorator, type ServiceIdentifier } from '#/_base/di/instantiation';
22

3-
export interface ProjectAdditionalDirsLoadResult {
3+
export interface ProjectAdditionalDirsLocation {
44
readonly projectRoot: string;
55
readonly configPath: string;
6+
}
7+
8+
export interface ProjectAdditionalDirsLoadResult extends ProjectAdditionalDirsLocation {
69
readonly additionalDirs: readonly string[];
710
}
811

912
export interface IProjectLocalConfigService {
1013
readonly _serviceBrand: undefined;
1114

15+
locateAdditionalDirsConfig(workDir: string): Promise<ProjectAdditionalDirsLocation>;
1216
readAdditionalDirs(workDir: string): Promise<ProjectAdditionalDirsLoadResult>;
1317
resolveAdditionalDirs(baseDir: string, additionalDirs: readonly string[]): Promise<string[]>;
1418
appendAdditionalDir(

‎packages/agent-core-v2/src/persistence/backends/node-fs/projectLocalConfigService.ts‎

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@ import { IBootstrapService } from '#/app/bootstrap/bootstrap';
77
import {
88
IProjectLocalConfigService,
99
type ProjectAdditionalDirsLoadResult,
10+
type ProjectAdditionalDirsLocation,
1011
} from '#/app/projectLocalConfig/projectLocalConfig';
1112
import { ErrorCodes, Error2, unwrapErrorCause } from '#/errors';
1213
import { IHostFileSystem } from '#/os/interface/hostFileSystem';
1314
import { StorageError, StorageErrors, toStorageIoError } from '#/persistence/interface/storage';
15+
import { isWithinDirectory } from '#/tool/path-access';
1416

1517
const ProjectLocalTomlSchema = z.object({
1618
workspace: z
@@ -35,9 +37,13 @@ export class FileProjectLocalConfigService implements IProjectLocalConfigService
3537
@IHostFileSystem private readonly fs: IHostFileSystem,
3638
) {}
3739

38-
async readAdditionalDirs(workDir: string): Promise<ProjectAdditionalDirsLoadResult> {
40+
async locateAdditionalDirsConfig(workDir: string): Promise<ProjectAdditionalDirsLocation> {
3941
const projectRoot = await this.findProjectRoot(workDir);
40-
const configPath = this.getProjectLocalConfigPath(projectRoot);
42+
return { projectRoot, configPath: this.getProjectLocalConfigPath(projectRoot) };
43+
}
44+
45+
async readAdditionalDirs(workDir: string): Promise<ProjectAdditionalDirsLoadResult> {
46+
const { projectRoot, configPath } = await this.locateAdditionalDirsConfig(workDir);
4147
const file = await this.readProjectLocalToml(configPath);
4248

4349
const additionalDirs = file?.parsed.workspace?.additional_dir;
@@ -65,7 +71,7 @@ export class FileProjectLocalConfigService implements IProjectLocalConfigService
6571
const additionalDir = await this.resolveAdditionalDir(workDir, inputPath);
6672
const file = (await this.readProjectLocalToml(configPath)) ?? { raw: {}, parsed: {} };
6773
const fileAdditionalDirs = file.parsed.workspace?.additional_dir ?? [];
68-
const fileExistingDirs = this.resolveExistingAdditionalDirs(
74+
const fileExistingDirs = await this.resolveExistingAdditionalDirs(
6975
projectRoot,
7076
fileAdditionalDirs,
7177
);
@@ -156,14 +162,14 @@ export class FileProjectLocalConfigService implements IProjectLocalConfigService
156162
return resolvedDirs;
157163
}
158164

159-
private resolveExistingAdditionalDirs(
165+
private async resolveExistingAdditionalDirs(
160166
projectRoot: string,
161167
additionalDirs: readonly string[],
162-
): string[] {
168+
): Promise<string[]> {
163169
const resolvedDirs: string[] = [];
164170

165171
for (const additionalDir of normalizeAdditionalDirs(additionalDirs)) {
166-
const resolvedDir = this.resolvePath(projectRoot, additionalDir);
172+
const resolvedDir = await this.resolvePath(projectRoot, additionalDir);
167173
if (this.hasSameAdditionalDir(resolvedDirs, resolvedDir)) continue;
168174
resolvedDirs.push(resolvedDir);
169175
}
@@ -176,14 +182,38 @@ export class FileProjectLocalConfigService implements IProjectLocalConfigService
176182
additionalDir: string,
177183
): Promise<string> {
178184
const normalizedInput = normalizeAdditionalDirInput(additionalDir);
179-
const resolvedDir = this.resolvePath(baseDir, normalizedInput);
185+
const resolvedDir = await this.resolvePath(baseDir, normalizedInput);
180186
await this.assertDirectory(resolvedDir);
181187
return resolvedDir;
182188
}
183189

184-
private resolvePath(baseDir: string, additionalDir: string): string {
190+
private async resolvePath(baseDir: string, additionalDir: string): Promise<string> {
185191
const expanded = this.expandHome(additionalDir);
186-
return isAbsolute(expanded) ? normalize(expanded) : resolve(baseDir, expanded);
192+
const resolvedDir = isAbsolute(expanded) ? normalize(expanded) : resolve(baseDir, expanded);
193+
if (await this.isBroadScopeDir(resolvedDir)) {
194+
throw new Error2(
195+
ErrorCodes.CONFIG_INVALID,
196+
'workspace.additional_dir must not be the user home directory or the filesystem root',
197+
);
198+
}
199+
return resolvedDir;
200+
}
201+
202+
private async isBroadScopeDir(resolvedDir: string): Promise<boolean> {
203+
const homeDir = normalize(this.bootstrap.osHomeDir);
204+
if (dirname(resolvedDir) === resolvedDir) return true;
205+
const realDir = await this.realpathOrLexical(resolvedDir);
206+
if (dirname(realDir) === realDir) return true;
207+
const realHome = await this.realpathOrLexical(homeDir);
208+
return isWithinDirectory(homeDir, resolvedDir) || isWithinDirectory(realHome, realDir);
209+
}
210+
211+
private async realpathOrLexical(path: string): Promise<string> {
212+
try {
213+
return normalize(await this.fs.realpath(path));
214+
} catch {
215+
return path;
216+
}
187217
}
188218

189219
private expandHome(value: string): string {

‎packages/agent-core-v2/src/program/program.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -283,11 +283,11 @@ export class Program {
283283
try {
284284
const state = own(new WorkspaceStateService(this.dependencies.appState));
285285
const localConfig = new FileProjectLocalConfigService(this.dependencies.bootstrap, runtime.fs!);
286-
const dirs = own(new WorkspaceDirsService(this.context, localConfig, this.dependencies.log, state));
286+
const trust = own(new WorkspaceTrustService(this.context, this.dependencies.docs, state, this.dependencies.telemetry, this.dependencies.bootstrap));
287+
const dirs = own(new WorkspaceDirsService(this.context, localConfig, this.dependencies.log, state, trust));
287288
const git = new WorkspaceGitService(this.context, this.dependencies.git);
288289
const fs = new WorkspaceFsService(this.context, dirs, runtime.fs!, this.resolver, this.dependencies.telemetry, git);
289290
const instructions = own(new WorkspaceInstructionsService(this.context, runtime.fs!, runtime.environment, this.dependencies.bootstrap, this.dependencies.log, state));
290-
const trust = own(new WorkspaceTrustService(this.context, this.dependencies.docs, state, this.dependencies.telemetry, this.dependencies.bootstrap));
291291
const mcpConfig = own(new WorkspaceMcpConfigService(this.context, this.dependencies.bootstrap, this.dependencies.plugins, this.dependencies.log, this.dependencies.config, runtime.fs!, trust, this.dependencies.configStore));
292292
const mcp = own(new WorkspaceMcpService(this.context, this.resolver, mcpConfig, this.dependencies.oauth, this.dependencies.log, this.dependencies.telemetry, this.dependencies.identity, this.dependencies.sessionManager));
293293
const userAgentProfiles = own(new UserAgentProfileLoaderService(this.dependencies.bootstrap, runtime.fs!, this.dependencies.log, this.dependencies.builtinAgentProfiles, this.context, this.dependencies.agentProfiles));

‎packages/agent-core-v2/src/session/agentLifecycle/profile/gitContext.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,6 @@ async function runGit(
203203
await proc.kill('SIGKILL');
204204
} catch {
205205
}
206-
await work.catch(() => {});
207206
if (timedOut) return { ok: false, kind: 'timeout' };
208207
return { ok: false, kind: 'command-failed' };
209208
} finally {

‎packages/agent-core-v2/src/workspace/workspaceDirs/workspaceDirsService.ts‎

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@ import { TimeoutTimer } from '#/_base/utils/timer';
66
import { subtreeWatchFilter } from '#/_base/utils/paths';
77
import {
88
IProjectLocalConfigService,
9+
type ProjectAdditionalDirsLoadResult,
910
} from '#/app/projectLocalConfig/projectLocalConfig';
1011
import type { ISessionWorkspaceInfo } from '#/session/workspaceInfo/workspaceInfo';
1112
import { IWorkspaceStateService } from '#/workspace/state/workspaceState';
1213
import { IWorkspaceContext } from '#/workspace/workspaceContext/workspaceContext';
14+
import { IWorkspaceTrust } from '#/workspace/workspaceTrust/workspaceTrust';
1315
import { watchCandidates } from '#human/utils/watch';
1416

1517
import {
@@ -45,14 +47,28 @@ export class WorkspaceDirsService extends Disposable implements IWorkspaceDirs {
4547
@IProjectLocalConfigService private readonly localConfig: IProjectLocalConfigService,
4648
@ILogService private readonly log: ILogService,
4749
@IWorkspaceStateService private readonly states: IWorkspaceStateService,
50+
@IWorkspaceTrust private readonly trust: IWorkspaceTrust,
4851
) {
4952
super();
5053
this.states.contributeState(workspaceDirsFileDirsKey);
5154
this.states.contributeState(workspaceDirsEphemeralDirsKey);
5255
this.projectRoot = workspace.cwd;
5356
this.configPath = '';
5457
this.ready = this.enqueue(() => this.reloadFromDisk());
55-
void this.ready.then(() => this.watchLocalToml());
58+
this.ready.then(
59+
() => this.watchLocalToml(),
60+
() => {},
61+
);
62+
this._register(
63+
this.trust.onDidChange(() => {
64+
if (!this.trust.isTrusted() && this.setFileDirs([])) {
65+
this.onDidChangeEmitter.fire();
66+
}
67+
void this.enqueue(() => this.reloadFromDisk()).catch((error) => {
68+
this.log.warn(`local.toml trust reload failed: ${String(error)}`);
69+
});
70+
}),
71+
);
5672
}
5773

5874
private get fileDirs(): readonly string[] {
@@ -111,7 +127,15 @@ export class WorkspaceDirsService extends Disposable implements IWorkspaceDirs {
111127
);
112128
this.projectRoot = persisted.projectRoot;
113129
this.configPath = persisted.configPath;
114-
const changed = this.setFileDirs(persisted.additionalDirs);
130+
let changed: boolean;
131+
if (this.trust.isTrusted()) {
132+
changed = this.setFileDirs(persisted.additionalDirs);
133+
} else {
134+
const explicit = await this.localConfig.resolveAdditionalDirs(this.workspace.cwd, [
135+
input.path,
136+
]);
137+
changed = this.unionEphemeral(explicit);
138+
}
115139
if (changed) {
116140
this.onDidChangeEmitter.fire();
117141
}
@@ -123,7 +147,7 @@ export class WorkspaceDirsService extends Disposable implements IWorkspaceDirs {
123147
};
124148
}
125149

126-
const onDisk = await this.localConfig.readAdditionalDirs(this.workspace.cwd);
150+
const onDisk = await this.localConfig.locateAdditionalDirsConfig(this.workspace.cwd);
127151
this.projectRoot = onDisk.projectRoot;
128152
this.configPath = onDisk.configPath;
129153
const resolved = await this.localConfig.resolveAdditionalDirs(this.workspace.cwd, [
@@ -142,10 +166,18 @@ export class WorkspaceDirsService extends Disposable implements IWorkspaceDirs {
142166
}
143167

144168
private async reloadFromDisk(): Promise<void> {
145-
const onDisk = await this.localConfig.readAdditionalDirs(this.workspace.cwd);
169+
await this.trust.ready;
170+
const trustedAtStart = this.trust.isTrusted();
171+
const onDisk: ProjectAdditionalDirsLoadResult = trustedAtStart
172+
? await this.localConfig.readAdditionalDirs(this.workspace.cwd)
173+
: {
174+
...(await this.localConfig.locateAdditionalDirsConfig(this.workspace.cwd)),
175+
additionalDirs: [],
176+
};
146177
this.projectRoot = onDisk.projectRoot;
147178
this.configPath = onDisk.configPath;
148-
if (this.setFileDirs(onDisk.additionalDirs)) {
179+
const dirs = this.trust.isTrusted() ? onDisk.additionalDirs : [];
180+
if (this.setFileDirs(dirs)) {
149181
this.onDidChangeEmitter.fire();
150182
}
151183
}

0 commit comments

Comments
 (0)