Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-kap-server-auth-encoded-path.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@moonshot-ai/kimi-code": patch
---

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.
5 changes: 5 additions & 0 deletions .changeset/fix-sessionfs-symlink-escape.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@moonshot-ai/kimi-code": patch
---

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.
5 changes: 5 additions & 0 deletions .changeset/fix-symlinked-workspace-root.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@moonshot-ai/kimi-code": patch
---

Fix sessions failing to be created when the workspace directory is given through a symlink, which the v2 engine rejected as "not a directory".
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,12 @@
* `createOrTouch` is the single choke point every workspace/session creation
* funnels through, so it owns the root-existence contract: the root must be
* an existing directory on the host filesystem, otherwise it throws
* `fs.path_not_found` (mirrors v1's `WorkspaceRootNotFoundError`). The rebuild
* and merge paths bypass the check on purpose — they catalog where sessions
* *were*, not where new ones may open. Bound at App scope.
* `fs.path_not_found` (mirrors v1's `WorkspaceRootNotFoundError`). The
* directory probe follows symlinks (`IHostFileSystem.stat` is lstat-based, so
* a symlink-form root is re-checked through `realpath`), while the workspace
* identity stays lexical — v1 deliberately never realpaths the root either.
* The rebuild and merge paths bypass the check on purpose — they catalog
* where sessions *were*, not where new ones may open. Bound at App scope.
*/

import { basename, isAbsolute } from 'pathe';
Expand Down Expand Up @@ -101,6 +104,13 @@ export class WorkspaceRegistryService implements IWorkspaceRegistry {
}
throw error;
}
if (!stat.isDirectory) {
try {
stat = await this.hostFs.stat(await this.hostFs.realpath(root));
} catch {
// Fall through to the not-a-directory error below.
}
}
if (!stat.isDirectory) {
throw new Error2(ErrorCodes.FS_PATH_NOT_FOUND, `workspace root ${root} is not a directory`);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,17 @@
* Bound at App scope.
*/

import { appendFile, lstat, open, readFile, readdir, mkdir, rm, writeFile } from 'node:fs/promises';
import {
appendFile,
lstat,
open,
readFile,
readdir,
mkdir,
realpath as nodeRealpath,
rm,
writeFile,
} from 'node:fs/promises';

import { InstantiationType } from '#/_base/di/extensions';
import { LifecycleScope, registerScopedService } from '#/_base/di/scope';
Expand Down Expand Up @@ -222,6 +232,14 @@ export class HostFileSystem implements IHostFileSystem {
throw toHostFsError(error, { path, op: 'remove' });
}
}

async realpath(path: string): Promise<string> {
try {
return await nodeRealpath(path);
} catch (error) {
throw toHostFsError(error, { path, op: 'realpath' });
}
}
}

registerScopedService(
Expand Down
7 changes: 5 additions & 2 deletions packages/agent-core-v2/src/os/interface/hostFileSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
*
* Defines the `IHostFileSystem` used by the program side (persistence, skill
* loading, workspace registry) and the os file tools to read and write files on
* the real local disk, plus the stat/entry models. App-scoped — one shared
* instance.
* the real local disk, plus the stat/entry models. `realpath` canonicalizes a
* path by resolving every symlink component (Node `fs.realpath` semantics) and
* rejects with `os.fs.not_found` for a missing path; consumers use it to make
* lexical path confinement symlink-aware. App-scoped — one shared instance.
*/

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

export const IHostFileSystem: ServiceIdentifier<IHostFileSystem> =
Expand Down
101 changes: 84 additions & 17 deletions packages/agent-core-v2/src/session/sessionFs/fsService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@
* `IGitService`; this service only confines paths and computes repo-relative
* paths before calling it.
*
* Path confinement is lexical (`ISessionWorkspaceContext.isWithin`); it does not
* follow symlinks, matching the rest of v2 (`_base/tools/policies/path-access.ts`).
* Path confinement applies the lexical `ISessionWorkspaceContext.isWithin`
* check first, then re-verifies the candidate through `IHostFileSystem.realpath`
* (resolving the longest existing prefix, so not-yet-created paths still work):
* a symlink inside the workspace must not steer fs actions to files outside it.
*/

import { basename, extname, isAbsolute, join, relative, sep } from 'node:path';
import { basename, dirname, extname, isAbsolute, join, relative, sep } from 'node:path';

import {
ErrorCode,
Expand Down Expand Up @@ -46,7 +48,7 @@ import ignore, { type Ignore } from 'ignore';

import { InstantiationType } from '#/_base/di/extensions';
import { LifecycleScope, registerScopedService } from '#/_base/di/scope';
import { ErrorCodes, Error2, unwrapErrorCause } from '#/errors';
import { ErrorCodes, Error2, isError2, unwrapErrorCause } from '#/errors';
import { IGitService } from '#/app/git/git';
import { ITelemetryService } from '#/app/telemetry/telemetry';
import { IHostFileSystem, type HostDirEntry, type HostFileStat } from '#/os/interface/hostFileSystem';
Expand Down Expand Up @@ -97,7 +99,7 @@ export class SessionFsService implements ISessionFsService {
}

async list(req: FsListRequest): Promise<FsListResponse> {
const abs = this.resolveWithin(req.path);
const abs = await this.resolveWithin(req.path);
const rel = this.toRel(abs);

let topStat: HostFileStat;
Expand Down Expand Up @@ -189,7 +191,7 @@ export class SessionFsService implements ISessionFsService {
}

async read(req: FsReadRequest): Promise<FsReadResponse> {
const abs = this.resolveWithin(req.path);
const abs = await this.resolveWithin(req.path);
const rel = this.toRel(abs);

let st: HostFileStat;
Expand Down Expand Up @@ -289,7 +291,7 @@ export class SessionFsService implements ISessionFsService {
}

async stat(req: FsStatRequest): Promise<FsStatResponse> {
const abs = this.resolveWithin(req.path);
const abs = await this.resolveWithin(req.path);
const rel = this.toRel(abs);
let st: HostFileStat;
try {
Expand All @@ -302,10 +304,12 @@ export class SessionFsService implements ISessionFsService {
}

async statMany(req: FsStatManyRequest): Promise<FsStatManyResponse> {
const resolved = req.paths.map((p) => {
const abs = this.resolveWithin(p);
return { raw: p, rel: this.toRel(abs), abs };
});
const resolved = await Promise.all(
req.paths.map(async (p) => {
const abs = await this.resolveWithin(p);
return { raw: p, rel: this.toRel(abs), abs };
}),
);

const entries: Record<string, FsEntry | null> = {};
await Promise.all(
Expand All @@ -323,7 +327,7 @@ export class SessionFsService implements ISessionFsService {
}

async mkdir(req: FsMkdirRequest): Promise<FsMkdirResponse> {
const abs = this.resolveWithin(req.path);
const abs = await this.resolveWithin(req.path);
const rel = this.toRel(abs);
try {
await this.hostFs.mkdir(abs, { recursive: req.recursive });
Expand All @@ -346,7 +350,7 @@ export class SessionFsService implements ISessionFsService {
}

async resolvePath(relPath: string): Promise<FsPathResolved> {
const abs = this.resolveWithin(relPath);
const abs = await this.resolveWithin(relPath);
const rel = this.toRel(abs);
let st: HostFileStat;
try {
Expand All @@ -358,7 +362,7 @@ export class SessionFsService implements ISessionFsService {
}

async resolveDownload(relPath: string): Promise<FsDownloadResolved> {
const abs = this.resolveWithin(relPath);
const abs = await this.resolveWithin(relPath);
const rel = this.toRel(abs);
let st: HostFileStat;
try {
Expand Down Expand Up @@ -442,7 +446,7 @@ export class SessionFsService implements ISessionFsService {
if (req.paths !== undefined && req.paths.length > 0) {
filter = new Set();
for (const p of req.paths) {
filter.add(this.toRel(this.resolveWithin(p)));
filter.add(this.toRel(await this.resolveWithin(p)));
}
}

Expand All @@ -451,7 +455,7 @@ export class SessionFsService implements ISessionFsService {

async diff(req: FsDiffRequest): Promise<FsDiffResponse> {
const cwd = this.workspace.workDir;
const abs = this.resolveWithin(req.path);
const abs = await this.resolveWithin(req.path);
return this.git.diff(cwd, this.toRel(abs), abs);
}

Expand Down Expand Up @@ -669,7 +673,43 @@ export class SessionFsService implements ISessionFsService {
return this.rgResolution;
}

private resolveWithin(inputPath: string): string {
private realRootsCache: { readonly key: string; readonly roots: readonly string[] } | undefined;

private async realRoots(): Promise<readonly string[]> {
const dirs = [this.workspace.workDir, ...this.workspace.additionalDirs];
const key = dirs.join('\n');
if (this.realRootsCache?.key === key) return this.realRootsCache.roots;
const roots: string[] = [];
for (const dir of dirs) {
try {
roots.push(await this.hostFs.realpath(dir));
} catch {
roots.push(dir);
}
}
this.realRootsCache = { key, roots };
return roots;
}

private async realpathExistingPrefix(abs: string): Promise<string> {
const tail: string[] = [];
let current = abs;
for (let i = 0; i < 256; i++) {
try {
const real = await this.hostFs.realpath(current);
return tail.length === 0 ? real : join(real, ...tail.reverse());
} catch (err) {
if (!isMissingPathError(err)) throw err;
const parent = dirname(current);
if (parent === current) return abs;
tail.push(basename(current));
current = parent;
}
}
return abs;
}

private async resolveWithin(inputPath: string): Promise<string> {
if (inputPath === '' || inputPath === '/') {
throw new Error2(ErrorCodes.FS_PATH_ESCAPES, `path "${inputPath}" rejected (empty)`, {
details: { path: inputPath, reason: 'empty' },
Expand All @@ -692,6 +732,15 @@ export class SessionFsService implements ISessionFsService {
details: { path: inputPath, reason: 'resolved_outside' },
});
}
const resolved = await this.realpathExistingPrefix(abs);
const roots = await this.realRoots();
if (!roots.some((root) => isInsideOrEqual(resolved, root))) {
throw new Error2(
ErrorCodes.FS_PATH_ESCAPES,
`path "${inputPath}" escapes workspace through a symlink`,
{ details: { path: inputPath, reason: 'symlink_outside' } },
);
}
return abs;
}

Expand Down Expand Up @@ -905,6 +954,24 @@ function errnoCode(err: unknown): string | undefined {
return undefined;
}

function isMissingPathError(err: unknown): boolean {
if (isError2(err)) {
return (
err.code === ErrorCodes.OS_FS_NOT_FOUND || err.code === ErrorCodes.OS_FS_NOT_DIRECTORY
);
}
const code = errnoCode(err);
return code === 'ENOENT' || code === 'ENOTDIR';
}

function isInsideOrEqual(child: string, parent: string): boolean {
const rel = relative(parent, child);
if (rel === '') return true;
if (rel.startsWith('..')) return false;
if (isAbsolute(rel)) return false;
return true;
}

function mapFsError(err: unknown, inputPath: string): Error {
const code = errnoCode(err);
if (code === 'ENOENT' || code === 'ENOTDIR') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,16 @@ describe('WorkspaceRegistryService (file-backed)', () => {
expect(await build().list()).toEqual([]);
});

it('accepts createOrTouch when the root is given through a symlink', async () => {
const real = join(homeDir, 'real-root');
await fsp.mkdir(real, { recursive: true });
const link = join(homeDir, 'link-root');
await fsp.symlink(real, link, 'dir');
const ws = await build().createOrTouch(link);
expect(ws.root).toBe(link);
expect(ws.id).toBe(encodeWorkDirKey(link));
});

it('rejects createOrTouch when a parent of the root is not a directory', async () => {
const file = join(homeDir, 'a-file.txt');
await fsp.writeFile(file, 'hi', 'utf8');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ function createTestFs(kaos: FakeKaos): IHostFileSystem {
readdir: () => notImplemented('readdir'),
mkdir: () => notImplemented('mkdir'),
remove: () => notImplemented('remove'),
realpath: () => notImplemented('realpath'),
};
}

Expand Down
Loading
Loading