Skip to content

Commit d8ddabb

Browse files
authored
fix(kap-server): close auth bypass via percent-encoded API paths (#1753)
* fix(kap-server): fail closed on percent-encoded API paths in auth bypass The auth hook checked the raw, still percent-encoded request URL against its bypass policy, while find-my-way matches routes against the decoded path. A raw /%61pi/v1/... URL therefore skipped the bearer-token check yet still reached the /api/... handlers, allowing unauthenticated access to every API route. Decode the request path the same way the router does before matching, and fail closed when the path cannot be decoded. Add tests covering encoded /api/ paths, encoded meta documents, the encoded healthz probe, and unaffected non-API bypasses. * fix(agent-core-v2): make session fs path confinement symlink-aware SessionFsService confined paths only lexically, so a symlink planted inside the workspace could steer read, list, stat, mkdir, resolvePath and resolveDownload to host files outside the session directory. - add IHostFileSystem.realpath (Node fs.realpath semantics) and implement it in the node-local HostFileSystem - resolveWithin now re-verifies each candidate through realpath of the longest existing prefix (so not-yet-created paths still work) and rejects paths escaping the canonicalized workspace roots, which are resolved once and cached - cover symlink escapes for every fs entry point plus kap-server read/download e2e; in-workspace symlink targets remain allowed * fix(agent-core-v2): accept workspace roots given through a symlink WorkspaceRegistryService.createOrTouch probes the root with the lstat-based IHostFileSystem.stat, so a root given as a symlink to a directory reported isDirectory=false and session creation was rejected with fs.path_not_found ("not a directory"). Re-check a non-directory root through IHostFileSystem.realpath before failing, while keeping the workspace identity lexical (v1 deliberately never realpaths the root either). Covered by a unit test for a symlink-form root and a kap-server e2e that creates a session with a symlinked cwd and reads a file through it.
1 parent 4f99114 commit d8ddabb

15 files changed

Lines changed: 386 additions & 27 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@moonshot-ai/kimi-code": patch
3+
---
4+
5+
Fix the web server bearer-token check being bypassed by percent-encoded API paths (e.g. `/%61pi/v1/…`), which allowed unauthenticated access to every API route.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@moonshot-ai/kimi-code": patch
3+
---
4+
5+
Fix the session filesystem API following symlinks that point outside the workspace, which allowed reading, listing, creating, and downloading host files beyond the session directory through a planted symlink.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@moonshot-ai/kimi-code": patch
3+
---
4+
5+
Fix sessions failing to be created when the workspace directory is given through a symlink, which the v2 engine rejected as "not a directory".

packages/agent-core-v2/src/app/workspaceRegistry/workspaceRegistryService.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,12 @@
3131
* `createOrTouch` is the single choke point every workspace/session creation
3232
* funnels through, so it owns the root-existence contract: the root must be
3333
* an existing directory on the host filesystem, otherwise it throws
34-
* `fs.path_not_found` (mirrors v1's `WorkspaceRootNotFoundError`). The rebuild
35-
* and merge paths bypass the check on purpose — they catalog where sessions
36-
* *were*, not where new ones may open. Bound at App scope.
34+
* `fs.path_not_found` (mirrors v1's `WorkspaceRootNotFoundError`). The
35+
* directory probe follows symlinks (`IHostFileSystem.stat` is lstat-based, so
36+
* a symlink-form root is re-checked through `realpath`), while the workspace
37+
* identity stays lexical — v1 deliberately never realpaths the root either.
38+
* The rebuild and merge paths bypass the check on purpose — they catalog
39+
* where sessions *were*, not where new ones may open. Bound at App scope.
3740
*/
3841

3942
import { basename, isAbsolute } from 'pathe';
@@ -101,6 +104,13 @@ export class WorkspaceRegistryService implements IWorkspaceRegistry {
101104
}
102105
throw error;
103106
}
107+
if (!stat.isDirectory) {
108+
try {
109+
stat = await this.hostFs.stat(await this.hostFs.realpath(root));
110+
} catch {
111+
// Fall through to the not-a-directory error below.
112+
}
113+
}
104114
if (!stat.isDirectory) {
105115
throw new Error2(ErrorCodes.FS_PATH_NOT_FOUND, `workspace root ${root} is not a directory`);
106116
}

packages/agent-core-v2/src/os/backends/node-local/hostFsService.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,17 @@
55
* Bound at App scope.
66
*/
77

8-
import { appendFile, lstat, open, readFile, readdir, mkdir, rm, writeFile } from 'node:fs/promises';
8+
import {
9+
appendFile,
10+
lstat,
11+
open,
12+
readFile,
13+
readdir,
14+
mkdir,
15+
realpath as nodeRealpath,
16+
rm,
17+
writeFile,
18+
} from 'node:fs/promises';
919

1020
import { InstantiationType } from '#/_base/di/extensions';
1121
import { LifecycleScope, registerScopedService } from '#/_base/di/scope';
@@ -222,6 +232,14 @@ export class HostFileSystem implements IHostFileSystem {
222232
throw toHostFsError(error, { path, op: 'remove' });
223233
}
224234
}
235+
236+
async realpath(path: string): Promise<string> {
237+
try {
238+
return await nodeRealpath(path);
239+
} catch (error) {
240+
throw toHostFsError(error, { path, op: 'realpath' });
241+
}
242+
}
225243
}
226244

227245
registerScopedService(

packages/agent-core-v2/src/os/interface/hostFileSystem.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
*
44
* Defines the `IHostFileSystem` used by the program side (persistence, skill
55
* loading, workspace registry) and the os file tools to read and write files on
6-
* the real local disk, plus the stat/entry models. App-scoped — one shared
7-
* instance.
6+
* the real local disk, plus the stat/entry models. `realpath` canonicalizes a
7+
* path by resolving every symlink component (Node `fs.realpath` semantics) and
8+
* rejects with `os.fs.not_found` for a missing path; consumers use it to make
9+
* lexical path confinement symlink-aware. App-scoped — one shared instance.
810
*/
911

1012
import { createDecorator, type ServiceIdentifier } from '#/_base/di/instantiation';
@@ -46,6 +48,7 @@ export interface IHostFileSystem {
4648
readdir(path: string): Promise<readonly HostDirEntry[]>;
4749
mkdir(path: string, options?: { readonly recursive?: boolean }): Promise<void>;
4850
remove(path: string): Promise<void>;
51+
realpath(path: string): Promise<string>;
4952
}
5053

5154
export const IHostFileSystem: ServiceIdentifier<IHostFileSystem> =

packages/agent-core-v2/src/session/sessionFs/fsService.ts

Lines changed: 84 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,13 @@
99
* `IGitService`; this service only confines paths and computes repo-relative
1010
* paths before calling it.
1111
*
12-
* Path confinement is lexical (`ISessionWorkspaceContext.isWithin`); it does not
13-
* follow symlinks, matching the rest of v2 (`_base/tools/policies/path-access.ts`).
12+
* Path confinement applies the lexical `ISessionWorkspaceContext.isWithin`
13+
* check first, then re-verifies the candidate through `IHostFileSystem.realpath`
14+
* (resolving the longest existing prefix, so not-yet-created paths still work):
15+
* a symlink inside the workspace must not steer fs actions to files outside it.
1416
*/
1517

16-
import { basename, extname, isAbsolute, join, relative, sep } from 'node:path';
18+
import { basename, dirname, extname, isAbsolute, join, relative, sep } from 'node:path';
1719

1820
import {
1921
type FsDiffRequest,
@@ -60,7 +62,7 @@ import ignore, { type Ignore } from 'ignore';
6062

6163
import { InstantiationType } from '#/_base/di/extensions';
6264
import { LifecycleScope, registerScopedService } from '#/_base/di/scope';
63-
import { ErrorCodes, Error2, unwrapErrorCause } from '#/errors';
65+
import { ErrorCodes, Error2, isError2, unwrapErrorCause } from '#/errors';
6466
import { IGitService } from '#/app/git/git';
6567
import { ITelemetryService } from '#/app/telemetry/telemetry';
6668
import { IHostFileSystem, type HostDirEntry, type HostFileStat } from '#/os/interface/hostFileSystem';
@@ -111,7 +113,7 @@ export class SessionFsService implements ISessionFsService {
111113
}
112114

113115
async list(req: FsListRequest): Promise<FsListResponse> {
114-
const abs = this.resolveWithin(req.path);
116+
const abs = await this.resolveWithin(req.path);
115117
const rel = this.toRel(abs);
116118

117119
let topStat: HostFileStat;
@@ -203,7 +205,7 @@ export class SessionFsService implements ISessionFsService {
203205
}
204206

205207
async read(req: FsReadRequest): Promise<FsReadResponse> {
206-
const abs = this.resolveWithin(req.path);
208+
const abs = await this.resolveWithin(req.path);
207209
const rel = this.toRel(abs);
208210

209211
let st: HostFileStat;
@@ -303,7 +305,7 @@ export class SessionFsService implements ISessionFsService {
303305
}
304306

305307
async stat(req: FsStatRequest): Promise<FsStatResponse> {
306-
const abs = this.resolveWithin(req.path);
308+
const abs = await this.resolveWithin(req.path);
307309
const rel = this.toRel(abs);
308310
let st: HostFileStat;
309311
try {
@@ -316,10 +318,12 @@ export class SessionFsService implements ISessionFsService {
316318
}
317319

318320
async statMany(req: FsStatManyRequest): Promise<FsStatManyResponse> {
319-
const resolved = req.paths.map((p) => {
320-
const abs = this.resolveWithin(p);
321-
return { raw: p, rel: this.toRel(abs), abs };
322-
});
321+
const resolved = await Promise.all(
322+
req.paths.map(async (p) => {
323+
const abs = await this.resolveWithin(p);
324+
return { raw: p, rel: this.toRel(abs), abs };
325+
}),
326+
);
323327

324328
const entries: Record<string, FsEntry | null> = {};
325329
await Promise.all(
@@ -337,7 +341,7 @@ export class SessionFsService implements ISessionFsService {
337341
}
338342

339343
async mkdir(req: FsMkdirRequest): Promise<FsMkdirResponse> {
340-
const abs = this.resolveWithin(req.path);
344+
const abs = await this.resolveWithin(req.path);
341345
const rel = this.toRel(abs);
342346
try {
343347
await this.hostFs.mkdir(abs, { recursive: req.recursive });
@@ -360,7 +364,7 @@ export class SessionFsService implements ISessionFsService {
360364
}
361365

362366
async resolvePath(relPath: string): Promise<FsPathResolved> {
363-
const abs = this.resolveWithin(relPath);
367+
const abs = await this.resolveWithin(relPath);
364368
const rel = this.toRel(abs);
365369
let st: HostFileStat;
366370
try {
@@ -372,7 +376,7 @@ export class SessionFsService implements ISessionFsService {
372376
}
373377

374378
async resolveDownload(relPath: string): Promise<FsDownloadResolved> {
375-
const abs = this.resolveWithin(relPath);
379+
const abs = await this.resolveWithin(relPath);
376380
const rel = this.toRel(abs);
377381
let st: HostFileStat;
378382
try {
@@ -456,7 +460,7 @@ export class SessionFsService implements ISessionFsService {
456460
if (req.paths !== undefined && req.paths.length > 0) {
457461
filter = new Set();
458462
for (const p of req.paths) {
459-
filter.add(this.toRel(this.resolveWithin(p)));
463+
filter.add(this.toRel(await this.resolveWithin(p)));
460464
}
461465
}
462466

@@ -465,7 +469,7 @@ export class SessionFsService implements ISessionFsService {
465469

466470
async diff(req: FsDiffRequest): Promise<FsDiffResponse> {
467471
const cwd = this.workspace.workDir;
468-
const abs = this.resolveWithin(req.path);
472+
const abs = await this.resolveWithin(req.path);
469473
return this.git.diff(cwd, this.toRel(abs), abs);
470474
}
471475

@@ -683,7 +687,43 @@ export class SessionFsService implements ISessionFsService {
683687
return this.rgResolution;
684688
}
685689

686-
private resolveWithin(inputPath: string): string {
690+
private realRootsCache: { readonly key: string; readonly roots: readonly string[] } | undefined;
691+
692+
private async realRoots(): Promise<readonly string[]> {
693+
const dirs = [this.workspace.workDir, ...this.workspace.additionalDirs];
694+
const key = dirs.join('\n');
695+
if (this.realRootsCache?.key === key) return this.realRootsCache.roots;
696+
const roots: string[] = [];
697+
for (const dir of dirs) {
698+
try {
699+
roots.push(await this.hostFs.realpath(dir));
700+
} catch {
701+
roots.push(dir);
702+
}
703+
}
704+
this.realRootsCache = { key, roots };
705+
return roots;
706+
}
707+
708+
private async realpathExistingPrefix(abs: string): Promise<string> {
709+
const tail: string[] = [];
710+
let current = abs;
711+
for (let i = 0; i < 256; i++) {
712+
try {
713+
const real = await this.hostFs.realpath(current);
714+
return tail.length === 0 ? real : join(real, ...tail.reverse());
715+
} catch (err) {
716+
if (!isMissingPathError(err)) throw err;
717+
const parent = dirname(current);
718+
if (parent === current) return abs;
719+
tail.push(basename(current));
720+
current = parent;
721+
}
722+
}
723+
return abs;
724+
}
725+
726+
private async resolveWithin(inputPath: string): Promise<string> {
687727
if (inputPath === '' || inputPath === '/') {
688728
throw new Error2(ErrorCodes.FS_PATH_ESCAPES, `path "${inputPath}" rejected (empty)`, {
689729
details: { path: inputPath, reason: 'empty' },
@@ -706,6 +746,15 @@ export class SessionFsService implements ISessionFsService {
706746
details: { path: inputPath, reason: 'resolved_outside' },
707747
});
708748
}
749+
const resolved = await this.realpathExistingPrefix(abs);
750+
const roots = await this.realRoots();
751+
if (!roots.some((root) => isInsideOrEqual(resolved, root))) {
752+
throw new Error2(
753+
ErrorCodes.FS_PATH_ESCAPES,
754+
`path "${inputPath}" escapes workspace through a symlink`,
755+
{ details: { path: inputPath, reason: 'symlink_outside' } },
756+
);
757+
}
709758
return abs;
710759
}
711760

@@ -919,6 +968,24 @@ function errnoCode(err: unknown): string | undefined {
919968
return undefined;
920969
}
921970

971+
function isMissingPathError(err: unknown): boolean {
972+
if (isError2(err)) {
973+
return (
974+
err.code === ErrorCodes.OS_FS_NOT_FOUND || err.code === ErrorCodes.OS_FS_NOT_DIRECTORY
975+
);
976+
}
977+
const code = errnoCode(err);
978+
return code === 'ENOENT' || code === 'ENOTDIR';
979+
}
980+
981+
function isInsideOrEqual(child: string, parent: string): boolean {
982+
const rel = relative(parent, child);
983+
if (rel === '') return true;
984+
if (rel.startsWith('..')) return false;
985+
if (isAbsolute(rel)) return false;
986+
return true;
987+
}
988+
922989
function mapFsError(err: unknown, inputPath: string): Error {
923990
const code = errnoCode(err);
924991
if (code === 'ENOENT' || code === 'ENOTDIR') {

packages/agent-core-v2/test/app/workspaceRegistry/workspaceRegistryService.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,16 @@ describe('WorkspaceRegistryService (file-backed)', () => {
382382
expect(await build().list()).toEqual([]);
383383
});
384384

385+
it('accepts createOrTouch when the root is given through a symlink', async () => {
386+
const real = join(homeDir, 'real-root');
387+
await fsp.mkdir(real, { recursive: true });
388+
const link = join(homeDir, 'link-root');
389+
await fsp.symlink(real, link, 'dir');
390+
const ws = await build().createOrTouch(link);
391+
expect(ws.root).toBe(link);
392+
expect(ws.id).toBe(encodeWorkDirKey(link));
393+
});
394+
385395
it('rejects createOrTouch when a parent of the root is not a directory', async () => {
386396
const file = join(homeDir, 'a-file.txt');
387397
await fsp.writeFile(file, 'hi', 'utf8');

packages/agent-core-v2/test/os/backends/node-local/tools/grep.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ function createTestFs(kaos: FakeKaos): IHostFileSystem {
142142
readdir: () => notImplemented('readdir'),
143143
mkdir: () => notImplemented('mkdir'),
144144
remove: () => notImplemented('remove'),
145+
realpath: () => notImplemented('realpath'),
145146
};
146147
}
147148

0 commit comments

Comments
 (0)