Skip to content

Commit eacceee

Browse files
fix: harden CI portability and bounded parsers
Accept ordinary Windows path aliases without relaxing junction defenses, replace uncontrolled regex parsing with linear scans, and make HTML escaping single-pass so the full Windows and CodeQL gates can run cleanly.
1 parent b60894f commit eacceee

11 files changed

Lines changed: 260 additions & 57 deletions

File tree

apps/daemon/src/services/effect-journal.ts

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -362,14 +362,7 @@ export class OperationEffectJournal {
362362

363363
private requireSafeDirectory(): string {
364364
fs.mkdirSync(this.stateRoot, { recursive: true, mode: 0o700 });
365-
const stat = fs.lstatSync(this.stateRoot);
366-
if (!stat.isDirectory() || stat.isSymbolicLink()) {
367-
throw new Error("Operation effect state directory is unsafe.");
368-
}
369-
const real = fs.realpathSync(this.stateRoot);
370-
if (comparablePath(real) !== comparablePath(this.stateRoot)) {
371-
throw new Error("Operation effect state directory traverses a link.");
372-
}
365+
assertLinkFreeDirectoryPath(this.stateRoot);
373366
return this.stateRoot;
374367
}
375368

@@ -644,12 +637,26 @@ function assertOpenedPathContained(directory: string, filePath: string): void {
644637
const realFile = fs.realpathSync(filePath);
645638
if (
646639
comparablePath(path.dirname(realFile)) !== comparablePath(realDirectory) ||
647-
path.basename(realFile) !== path.basename(filePath)
640+
comparablePath(path.basename(realFile)) !== comparablePath(path.basename(filePath))
648641
) {
649642
throw new Error("Operation effect file escaped its state directory.");
650643
}
651644
}
652645

646+
function assertLinkFreeDirectoryPath(target: string): void {
647+
const resolved = path.resolve(target);
648+
const root = path.parse(resolved).root;
649+
const relative = path.relative(root, resolved);
650+
let current = root;
651+
for (const component of relative.split(path.sep).filter(Boolean)) {
652+
current = path.join(current, component);
653+
const stat = fs.lstatSync(current);
654+
if (!stat.isDirectory() || stat.isSymbolicLink()) {
655+
throw new Error("Operation effect state directory traverses a link.");
656+
}
657+
}
658+
}
659+
653660
function assertSameOpenedFile(filePath: string, opened: ReturnType<typeof fs.fstatSync>): void {
654661
const current = fs.lstatSync(filePath);
655662
if (

apps/daemon/src/services/session-visibility.ts

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ export class SessionAuthorityStore {
5959
private appendQueue: Promise<void> = Promise.resolve();
6060

6161
constructor(options: SessionAuthorityOptions) {
62-
this.stateRoot = options.stateRoot ?? defaultAuthorityStateRoot();
62+
this.stateRoot = path.resolve(options.stateRoot ?? defaultAuthorityStateRoot());
6363
this.namespace = options.namespace;
6464
if (!/^[a-f0-9]{64}$/.test(this.namespace)) throw new Error("Invalid authority namespace.");
6565
this.suppliedKey = options.key ? Buffer.from(options.key) : undefined;
@@ -253,14 +253,7 @@ export class SessionAuthorityStore {
253253
}
254254

255255
private async requireExistingSafeDirectory(): Promise<string> {
256-
const stat = await fs.lstat(this.stateRoot);
257-
if (!stat.isDirectory() || stat.isSymbolicLink()) {
258-
throw new Error("Session authority state directory is unsafe.");
259-
}
260-
const real = await fs.realpath(this.stateRoot);
261-
if (comparablePath(real) !== comparablePath(path.resolve(this.stateRoot))) {
262-
throw new Error("Session authority state directory traverses a link.");
263-
}
256+
await assertLinkFreeDirectoryPath(this.stateRoot);
264257
return this.stateRoot;
265258
}
266259

@@ -390,11 +383,28 @@ async function rejectLinkIfPresent(filePath: string): Promise<void> {
390383

391384
async function assertOpenedPathContained(directory: string, filePath: string): Promise<void> {
392385
const [realDirectory, realFile] = await Promise.all([fs.realpath(directory), fs.realpath(filePath)]);
393-
if (path.dirname(realFile) !== realDirectory || path.basename(realFile) !== path.basename(filePath)) {
386+
if (
387+
comparablePath(path.dirname(realFile)) !== comparablePath(realDirectory) ||
388+
comparablePath(path.basename(realFile)) !== comparablePath(path.basename(filePath))
389+
) {
394390
throw new Error("Authority file escaped its state directory.");
395391
}
396392
}
397393

394+
async function assertLinkFreeDirectoryPath(target: string): Promise<void> {
395+
const resolved = path.resolve(target);
396+
const root = path.parse(resolved).root;
397+
const relative = path.relative(root, resolved);
398+
let current = root;
399+
for (const component of relative.split(path.sep).filter(Boolean)) {
400+
current = path.join(current, component);
401+
const stat = await fs.lstat(current);
402+
if (!stat.isDirectory() || stat.isSymbolicLink()) {
403+
throw new Error("Session authority state directory traverses a link.");
404+
}
405+
}
406+
}
407+
398408
function comparablePath(value: string): string {
399409
const normalized = path.normalize(value);
400410
return process.platform === "win32" ? normalized.toLowerCase() : normalized;

packages/assistant-runtime/src/openai-compatible.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,40 @@ import {
44
callAiProviderJson,
55
callOpenAiCompatibleJson,
66
checkAiProvider,
7+
openAiChatCompletionsUrl,
8+
openAiModelsUrl,
9+
parseJsonObjectFromText,
710
runWithAuthorizedProviderRequestForTesting
811
} from "./openai-compatible.js";
912

13+
test("provider URL normalization handles long slash runs without regex backtracking", () => {
14+
const slashes = "/".repeat(100_000);
15+
assert.equal(
16+
openAiChatCompletionsUrl(`http://127.0.0.1:1234/v1${slashes}`),
17+
"http://127.0.0.1:1234/v1/chat/completions"
18+
);
19+
assert.equal(
20+
openAiModelsUrl(`http://127.0.0.1:1234/v1/chat/completions${slashes}`),
21+
"http://127.0.0.1:1234/v1/models"
22+
);
23+
});
24+
25+
test("provider JSON parser extracts fenced objects with a linear delimiter scan", () => {
26+
assert.deepEqual(
27+
parseJsonObjectFromText('preface\n```JSON \n {"ok":true,"ticks":"``"} \n```\nafter'),
28+
{ ok: true, value: { ok: true, ticks: "``" } }
29+
);
30+
assert.deepEqual(
31+
parseJsonObjectFromText(`preface\n\`\`\`json\n${" ".repeat(100_000)}{"ok":true}\n\`\`\`\nafter`),
32+
{ ok: true, value: { ok: true } }
33+
);
34+
assert.deepEqual(
35+
parseJsonObjectFromText("```json\n{\"ok\":true}"),
36+
{ ok: true, value: { ok: true } },
37+
"an unterminated fence still falls back to balanced JSON extraction"
38+
);
39+
});
40+
1041
test("callOpenAiCompatibleJson accepts array-form message content", async (t) => {
1142
const originalFetch = globalThis.fetch;
1243
globalThis.fetch = (async () => new Response(JSON.stringify({

packages/assistant-runtime/src/openai-compatible.ts

Lines changed: 65 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -857,12 +857,15 @@ function openAiCompatibleDefaultEndpoint(
857857
}
858858

859859
function nativeApiBase(endpoint: string | undefined, fallback: string): string {
860-
const trimmed = (endpoint || fallback).trim().replace(/\/+$/g, "");
860+
const trimmed = stripTrailingSlashes((endpoint || fallback).trim());
861861
if (!trimmed) throw new Error("Provider endpoint is required.");
862-
return trimmed
863-
.replace(/\/v1(?:\/.*)?$/g, "")
864-
.replace(/\/api\/v1(?:\/.*)?$/g, "")
865-
.replace(/\/api(?:\/.*)?$/g, "");
862+
return stripPathFromSegment(
863+
stripPathFromSegment(
864+
stripPathFromSegment(trimmed, "/v1"),
865+
"/api/v1"
866+
),
867+
"/api"
868+
);
866869
}
867870

868871
function ollamaApiUrl(endpoint: string | undefined, route: "chat" | "tags"): string {
@@ -880,28 +883,31 @@ function requireAnthropicApiKey(apiKey: string | undefined): string {
880883
}
881884

882885
export function openAiChatCompletionsUrl(endpoint: string): string {
883-
const trimmed = endpoint.trim().replace(/\/+$/g, "");
886+
const trimmed = stripTrailingSlashes(endpoint.trim());
884887
if (!trimmed) throw new Error("OpenAI-compatible endpoint is required.");
885888
if (trimmed.endsWith("/chat/completions")) return trimmed;
886889
if (trimmed.endsWith("/v1")) return `${trimmed}/chat/completions`;
887890
return `${trimmed}/v1/chat/completions`;
888891
}
889892

890893
export function openAiModelsUrl(endpoint: string): string {
891-
const trimmed = endpoint.trim().replace(/\/+$/g, "");
894+
const trimmed = stripTrailingSlashes(endpoint.trim());
892895
if (!trimmed) throw new Error("OpenAI-compatible endpoint is required.");
893896
if (trimmed.endsWith("/models")) return trimmed;
894-
if (trimmed.endsWith("/chat/completions")) return `${trimmed.replace(/\/chat\/completions$/g, "")}/models`;
897+
if (trimmed.endsWith("/chat/completions")) {
898+
return `${trimmed.slice(0, -"/chat/completions".length)}/models`;
899+
}
895900
if (trimmed.endsWith("/v1")) return `${trimmed}/models`;
896901
return `${trimmed}/v1/models`;
897902
}
898903

899904
function providerModelsUrls(endpoint: string): string[] {
900-
const trimmed = endpoint.trim().replace(/\/+$/g, "");
905+
const trimmed = stripTrailingSlashes(endpoint.trim());
901906
const urls = [openAiModelsUrl(trimmed)];
902-
const withoutVersion = trimmed
903-
.replace(/\/v1(?:\/.*)?$/g, "")
904-
.replace(/\/api\/v1(?:\/.*)?$/g, "");
907+
const withoutVersion = stripPathFromSegment(
908+
stripPathFromSegment(trimmed, "/v1"),
909+
"/api/v1"
910+
);
905911
if (withoutVersion) {
906912
urls.push(`${withoutVersion}/api/v1/models`);
907913
urls.push(`${withoutVersion}/v1/models`);
@@ -934,10 +940,10 @@ export function parseJsonObjectFromText(input: string): { ok: true; value: unkno
934940
const trimmed = input.trim();
935941
if (!trimmed) return { ok: false, error: "empty response" };
936942

937-
const fenced = /```(?:json)?\s*([\s\S]*?)```/i.exec(trimmed);
943+
const fenced = extractMarkdownFence(trimmed);
938944
const candidates = [
939945
trimmed,
940-
fenced?.[1],
946+
fenced,
941947
extractBalancedJson(trimmed)
942948
].filter((candidate): candidate is string => Boolean(candidate?.trim()));
943949

@@ -956,6 +962,51 @@ export function parseJsonObjectFromText(input: string): { ok: true; value: unkno
956962
return { ok: false, error: "no parseable JSON object found" };
957963
}
958964

965+
/** Removes only terminal slash characters with one bounded reverse scan. */
966+
function stripTrailingSlashes(value: string): string {
967+
let end = value.length;
968+
while (end > 0 && value.charCodeAt(end - 1) === 47) end -= 1;
969+
return end === value.length ? value : value.slice(0, end);
970+
}
971+
972+
/**
973+
* Mirrors the former endpoint suffix normalization without an unanchored
974+
* wildcard expression. A segment qualifies only at the end or before another
975+
* slash, and the first qualifying segment wins just as left-to-right matching
976+
* did. Provider egress still crosses authorizedProviderFetch afterwards.
977+
*/
978+
function stripPathFromSegment(value: string, segment: string): string {
979+
let offset = 0;
980+
while (offset < value.length) {
981+
const index = value.indexOf(segment, offset);
982+
if (index < 0) return value;
983+
const boundary = index + segment.length;
984+
if (boundary === value.length || value.charCodeAt(boundary) === 47) {
985+
return value.slice(0, index);
986+
}
987+
offset = index + 1;
988+
}
989+
return value;
990+
}
991+
992+
/** Extracts the first Markdown fence in linear time without regex backtracking. */
993+
function extractMarkdownFence(value: string): string | undefined {
994+
const opening = value.indexOf("```");
995+
if (opening < 0) return undefined;
996+
let contentStart = opening + 3;
997+
if (value.slice(contentStart, contentStart + 4).toLowerCase() === "json") {
998+
contentStart += 4;
999+
}
1000+
while (
1001+
contentStart < value.length &&
1002+
value[contentStart]!.trim().length === 0
1003+
) {
1004+
contentStart += 1;
1005+
}
1006+
const closing = value.indexOf("```", contentStart);
1007+
return closing < 0 ? undefined : value.slice(contentStart, closing);
1008+
}
1009+
9591010
async function callOllamaText(
9601011
config: AiProviderConfig,
9611012
messages: AiChatMessage[],

packages/core/src/text.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import assert from "node:assert/strict";
2+
import test from "node:test";
3+
import { normalizeSlug } from "./text.js";
4+
5+
test("normalizeSlug trims boundary dashes while preserving allowed internal runs", () => {
6+
const internal = "-".repeat(32_768);
7+
assert.equal(
8+
normalizeSlug(`---Alpha${internal}Beta---`, {
9+
collapse: /[^a-z0-9-]+/g
10+
}),
11+
`alpha${internal}beta`
12+
);
13+
});

packages/core/src/text.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,19 @@ export function normalizeSlug(input: string | undefined, options: NormalizeSlugO
6262
.toLowerCase();
6363
if (options.strip) value = value.replace(options.strip, "");
6464
if (options.mapToDash) value = value.replace(options.mapToDash, "-");
65-
value = value
66-
.replace(options.collapse ?? /[^a-z0-9]+/g, "-")
67-
.replace(/^-+|-+$/g, "");
65+
value = value.replace(options.collapse ?? /[^a-z0-9]+/g, "-");
66+
value = trimAsciiDashes(value);
6867
return value || options.fallback || "";
6968
}
7069

70+
function trimAsciiDashes(input: string): string {
71+
let start = 0;
72+
let end = input.length;
73+
while (start < end && input.charCodeAt(start) === 45) start += 1;
74+
while (end > start && input.charCodeAt(end - 1) === 45) end -= 1;
75+
return start === 0 && end === input.length ? input : input.slice(start, end);
76+
}
77+
7178
/** Splits a comma-separated list, trimming items and dropping empties. */
7279
export function splitList(input: string): string[] {
7380
return input

packages/mcp-tools/src/install.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,22 @@ test("installs codex HTTP MCP config without auth token in none mode", async ()
2424
assert.doesNotMatch(config, /bearer_token_env_var/);
2525
});
2626

27+
test("normalizes a long trailing slash run in the daemon URL", async () => {
28+
const temp = await fs.mkdtemp(path.join(os.tmpdir(), "zharwing-mcp-install-"));
29+
const configPath = path.join(temp, "config.toml");
30+
31+
await installMcpClient({
32+
client: "codex",
33+
transport: "http",
34+
authMode: "none",
35+
daemonUrl: `http://127.0.0.1:37841${"/".repeat(32_768)}`,
36+
configPath
37+
});
38+
39+
const config = await fs.readFile(configPath, "utf8");
40+
assert.match(config, /url = "http:\/\/127\.0\.0\.1:37841\/mcp"/);
41+
});
42+
2743
test("installs the dedicated Zharwing agent credential for Codex HTTP", async () => {
2844
const temp = await fs.mkdtemp(path.join(os.tmpdir(), "zharwing-mcp-install-"));
2945
const configPath = path.join(temp, "config.toml");

packages/mcp-tools/src/install.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,9 @@ async function backupFile(filePath: string, contents: string): Promise<string> {
511511
}
512512

513513
function trimTrailingSlash(input: string): string {
514-
return input.replace(/\/+$/, "");
514+
let end = input.length;
515+
while (end > 0 && input.charCodeAt(end - 1) === 47) end -= 1;
516+
return end === input.length ? input : input.slice(0, end);
515517
}
516518

517519
function ensureTrailingNewline(input: string): string {

packages/storage/src/fs.test.ts

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import assert from "node:assert/strict";
2-
import test from "node:test";
3-
import { normalizeInteropPath } from "./fs.js";
2+
import { promises as fs } from "node:fs";
3+
import os from "node:os";
4+
import path from "node:path";
5+
import test, { type TestContext } from "node:test";
6+
import { atomicWriteText, normalizeInteropPath, readBoundedJson } from "./fs.js";
47

58
test("normalizeInteropPath maps WSL mount paths for a Windows daemon", () => {
69
assert.equal(
@@ -25,3 +28,36 @@ test("normalizeInteropPath leaves native paths unchanged", () => {
2528
assert.equal(normalizeInteropPath("/srv/project", "linux", false), "/srv/project");
2629
assert.equal(normalizeInteropPath("D:\\project", "win32", false), "D:\\project");
2730
});
31+
32+
test("safe storage accepts an ordinary Windows path through an extended-length alias", async (t) => {
33+
const ordinaryRoot = await tempRoot(t);
34+
const root = process.platform === "win32" ? path.toNamespacedPath(ordinaryRoot) : ordinaryRoot;
35+
const target = path.join(root, "records", "record.json");
36+
await atomicWriteText(target, '{"ok":true}\n', { root });
37+
assert.deepEqual(await readBoundedJson(target, { root, maximumBytes: 1_024 }), { ok: true });
38+
});
39+
40+
test("safe storage rejects a junction or symlink inside its owner root", async (t) => {
41+
const root = await tempRoot(t);
42+
const owner = path.join(root, "owner");
43+
const outside = path.join(root, "outside");
44+
const linked = path.join(owner, "linked");
45+
await fs.mkdir(owner);
46+
await fs.mkdir(outside);
47+
try {
48+
await fs.symlink(outside, linked, process.platform === "win32" ? "junction" : "dir");
49+
} catch (error) {
50+
if ((error as NodeJS.ErrnoException).code === "EPERM") return;
51+
throw error;
52+
}
53+
await assert.rejects(
54+
atomicWriteText(path.join(linked, "record.json"), "{}\n", { root: owner }),
55+
/traverses a filesystem link/
56+
);
57+
});
58+
59+
async function tempRoot(t: TestContext): Promise<string> {
60+
const root = await fs.mkdtemp(path.join(os.tmpdir(), "zharwing-storage-paths-"));
61+
t.after(() => fs.rm(root, { recursive: true, force: true }));
62+
return root;
63+
}

0 commit comments

Comments
 (0)