mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-14 02:29:33 +01:00
fix(custom-model): guard, document and test the session-supplied dsh home
Second review round on PR #755: skip non-absolute DSH_HOME candidates so a relative home can never mkdir under the server cwd; probe a session-supplied home with probePathKind in both async routes before the synchronous apply; test the session env-override lookup (seventh arg) incl. the self-link fall-through and the relative-home skip; reject '.' and '..' as realHomeFallback and enforce linkFromRealHomeDirs subset in the schema; fix the stale apply JSDoc and the two doc lines lagging the verified dsh run; state the shared/isolated split plainly.
This commit is contained in:
1 parent
1464332e91
commit
fa2b81484b
9 files changed
+189
-29
No files matched your search
@@ -599,14 +599,14 @@ const capabilitiesSchema = z
|
||||
// linking; missing files are skipped). Same segment pattern, see types.ts.
|
||||
linkFromRealHomeDirs: z.array(homeSegmentName).max(8).optional(),
|
||||
// The real home's fallback location, relative to the user's home: ONE segment —
|
||||
// a leading dot is legitimate (`.dsh` is what ships) but separators and `..`
|
||||
// are not; the consumer joins it under the user's home, and a traversal here
|
||||
// escapes that home.
|
||||
// a leading dot is legitimate (`.dsh` is what ships) but separators, `..` and
|
||||
// bare `.` are not; the consumer joins it under the user's home, a traversal
|
||||
// here escapes that home and `.` IS that home (linking THROUGH it unshielded).
|
||||
realHomeFallback: z
|
||||
.string()
|
||||
.min(1)
|
||||
.max(80)
|
||||
.regex(/^(?!\.\.\/?$)[^/\\]+$/)
|
||||
.regex(/^(?!\.\.?$)[^/\\]+$/)
|
||||
.optional(),
|
||||
})
|
||||
.strict()
|
||||
@@ -629,6 +629,19 @@ const capabilitiesSchema = z
|
||||
});
|
||||
}
|
||||
}
|
||||
// types.ts documents the dirs list as "must be a subset of linkFromRealHome";
|
||||
// a name absent from it names nothing (missing-file entries are skipped as
|
||||
// files), so the recipe would silently stop creating a directory it declares.
|
||||
const linkedNames = new Set(recipe.linkFromRealHome ?? []);
|
||||
for (const dir of recipe.linkFromRealHomeDirs ?? []) {
|
||||
if (!linkedNames.has(dir)) {
|
||||
ctx.addIssue({
|
||||
code: 'custom',
|
||||
message: `linkFromRealHomeDirs must be a subset of linkFromRealHome: ${JSON.stringify(dir)} is not linked, so the directory rule for it can never apply`,
|
||||
path: ['linkFromRealHomeDirs'],
|
||||
});
|
||||
}
|
||||
}
|
||||
}),
|
||||
z.object({ kind: z.literal('unsupported') }).strict(),
|
||||
]),
|
||||
|
||||
@@ -12,7 +12,7 @@
|
||||
|
||||
import { chmodSync, existsSync, lstatSync, mkdirSync, readFileSync, writeFileSync, rmSync, symlinkSync } from 'node:fs';
|
||||
import { homedir, platform } from 'node:os';
|
||||
import { join, dirname } from 'node:path';
|
||||
import { join, dirname, isAbsolute } from 'node:path';
|
||||
import { dataPath } from './config/instance.js';
|
||||
import type { CliEntry } from './config/cli-registry/types.js';
|
||||
import {
|
||||
@@ -30,9 +30,11 @@ export function customModelConfigDir(sessionId: string): string {
|
||||
* Writes a `ConfigDirInjection`'s files under `baseDir` and returns the full
|
||||
* envOverrides object a caller should merge into the session/process env
|
||||
* (the dir-redirect var plus any `extraEnv` the config file references by
|
||||
* name). Never touches anything outside `baseDir` — the caller is
|
||||
* responsible for choosing an isolated directory (never the user's real
|
||||
* `~/.codex`, `~/.pi`, etc.).
|
||||
* name). The generated files stay under `baseDir`, but a recipe that declares
|
||||
* `linkFromRealHome` additionally links (and, for missing directory targets,
|
||||
* creates) entries in the CLI's REAL home — the caller is still responsible
|
||||
* for choosing an isolated `baseDir` (never the user's real `~/.codex`,
|
||||
* `~/.pi`, etc.), it is just no longer the only directory touched.
|
||||
*
|
||||
* pi and omp embed the API key literally in the file, so the tree is written
|
||||
* 0700/0600 like every other secret-bearing file under `~/.codeman`; the chmod
|
||||
@@ -69,16 +71,21 @@ export function applyConfigDirInjection(
|
||||
*
|
||||
* Home resolution: the session's own override first (unless it IS `baseDir` — a re-apply
|
||||
* onto a session already holding a previous injection's value must not self-link), then
|
||||
* the server's environment, then the recipe's `realHomeFallback`. No home resolvable =
|
||||
* the server's environment, then the recipe's `realHomeFallback`. Only ABSOLUTE values
|
||||
* are considered: a set-but-relative `DSH_HOME`-style home resolves against the CLI's
|
||||
* cwd, and joining it here would mkdir/symlink under the SERVER's working directory and
|
||||
* leave relative links that dangle. No home resolvable =
|
||||
* no links, same best-effort posture as `linkSharedProjectsDir()`.
|
||||
*/
|
||||
function linkRealHomeEntries(baseDir: string, injection: ConfigDirInjection, sessionHomeOverride?: string): void {
|
||||
const names = injection.linkFromRealHome;
|
||||
if (!names || names.length === 0) return;
|
||||
const absoluteOnly = (value: string | undefined): string | undefined =>
|
||||
value && isAbsolute(value) ? value : undefined;
|
||||
const fromSession = sessionHomeOverride?.trim();
|
||||
const realHome =
|
||||
(fromSession && fromSession !== baseDir ? fromSession : undefined) ||
|
||||
process.env[injection.dirEnvVar]?.trim() ||
|
||||
absoluteOnly(fromSession && fromSession !== baseDir ? fromSession : undefined) ||
|
||||
absoluteOnly(process.env[injection.dirEnvVar]?.trim()) ||
|
||||
(injection.realHomeFallback ? join(homedir(), injection.realHomeFallback) : undefined);
|
||||
if (!realHome) return;
|
||||
const dirs = new Set(injection.linkFromRealHomeDirs ?? []);
|
||||
|
||||
@@ -243,6 +243,7 @@ function renderConfigFile(
|
||||
...(contextLength !== undefined && Number.isFinite(contextLength) && contextLength > 0
|
||||
? {
|
||||
contextWindow: Math.trunc(contextLength),
|
||||
// A quarter of the window keeps the reply inside the prompt's share; 8192 caps it on a large window.
|
||||
maxTokens: Math.max(1, Math.min(8192, Math.floor(contextLength / 4))),
|
||||
}
|
||||
: {}),
|
||||
|
||||
@@ -7,7 +7,7 @@
|
||||
import { claudeProjectDirName } from '../../case-rename.js';
|
||||
import { FastifyInstance, type FastifyRequest } from 'fastify';
|
||||
import { z } from 'zod';
|
||||
import { join, dirname, extname, basename, resolve as resolvePath } from 'node:path';
|
||||
import { join, dirname, extname, basename, isAbsolute, resolve as resolvePath } from 'node:path';
|
||||
import { homedir } from 'node:os';
|
||||
import { existsSync, statSync, mkdirSync, writeFileSync } from 'node:fs';
|
||||
import { execFile } from 'node:child_process';
|
||||
@@ -126,7 +126,7 @@ import { agentInbox } from '../agent-inbox.js';
|
||||
import { canUsernameRunPrivilegedCommands, resolveClaudeModeForUsername } from '../../user-store.js';
|
||||
import { clampEnvOverridesForOwner } from '../../session-env-clamp.js';
|
||||
import { enabledClis, getCli } from '../../config/cli-registry/registry.js';
|
||||
import type { AgentSkillDir, NewlineSequence } from '../../config/cli-registry/types.js';
|
||||
import type { AgentSkillDir, CliEntry, NewlineSequence } from '../../config/cli-registry/types.js';
|
||||
import { resolveCliLaunchError } from '../../utils/cli-launcher.js';
|
||||
import { applyLaunchDefaults } from '../launch-defaults.js';
|
||||
import { legacyConfigForMode } from '../../session-cli-registry-bridge.js';
|
||||
@@ -230,6 +230,36 @@ function landedInGroup(layout: TabLayout | undefined, sessionId: string): boolea
|
||||
return !!layout?.groups.some((group) => group.refs.some((ref) => ref.kind === 'session' && ref.id === sessionId));
|
||||
}
|
||||
|
||||
/**
|
||||
* Guard a session-supplied configDir home BEFORE `applyCustomModelInjection` runs.
|
||||
*
|
||||
* The apply resolves the CLI's real home from this session's env override and then runs
|
||||
* synchronous `existsSync`/`mkdirSync` on it (deepseek `DSH_HOME`, and any future
|
||||
* `linkFromRealHome` recipe). A path on an unreachable network mount makes a synchronous
|
||||
* stat wait forever, and this runs INSIDE a request handler, so it would freeze the whole
|
||||
* web server (CLAUDE.md, the #516 rule on user-chosen paths). A relative home is left to
|
||||
* the apply, which already refuses to join non-absolute values.
|
||||
*
|
||||
* Probe the session's own home with `probePathKind(..., { pastCap: true })` first: on
|
||||
* `'unknown'` (a dead mount that never answered) hand the apply a lookup that hides that
|
||||
* one env var, so linking falls through to the server's own environment and the unresponsive
|
||||
* path is never touched. Any other entry's lookup passes through untouched.
|
||||
*/
|
||||
async function guardSessionConfigDirHome(
|
||||
entry: Pick<CliEntry, 'capabilities'>,
|
||||
baseLookup: (envVar: string) => string | undefined
|
||||
): Promise<(envVar: string) => string | undefined> {
|
||||
const recipe = entry.capabilities.customModelInjection;
|
||||
if (recipe?.kind !== 'configDir' || !recipe.linkFromRealHome?.length) return baseLookup;
|
||||
const dirEnvVar = recipe.dirEnvVar;
|
||||
const supplied = baseLookup(dirEnvVar)?.trim();
|
||||
if (!supplied || !isAbsolute(supplied)) return baseLookup;
|
||||
if ((await probePathKind(supplied, { pastCap: true })) === 'unknown') {
|
||||
return (envVar: string) => (envVar === dirEnvVar ? undefined : baseLookup(envVar));
|
||||
}
|
||||
return baseLookup;
|
||||
}
|
||||
|
||||
/**
|
||||
* The case a POST /api/sessions `workingDir` belongs to, for its default session
|
||||
* group: a direct child of the caller's case space, or (admin / single-user, as
|
||||
@@ -1619,7 +1649,7 @@ export function registerSessionRoutes(
|
||||
session.id,
|
||||
contextLength,
|
||||
session.workingDir,
|
||||
(envVar) => session.envOverrideValue(envVar)
|
||||
await guardSessionConfigDirHome(entry, (envVar) => session.envOverrideValue(envVar))
|
||||
);
|
||||
if (!applied) {
|
||||
return createErrorResponse(ApiErrorCode.OPERATION_FAILED, `${session.mode} has no known custom-model mechanism`);
|
||||
@@ -4516,7 +4546,7 @@ export function registerSessionRoutes(
|
||||
qsCustomModelSessionId,
|
||||
cmContextLength,
|
||||
workspacePath,
|
||||
(envVar) => qsGatedEnvOverrides?.[envVar]
|
||||
await guardSessionConfigDirHome(cmEntry, (envVar) => qsGatedEnvOverrides?.[envVar])
|
||||
);
|
||||
if (!cmApplied) {
|
||||
return createErrorResponse(ApiErrorCode.OPERATION_FAILED, `${mode} has no known custom-model mechanism`);
|
||||
|
||||
Reference in new issue
Block a user