From abbad6ed520a197c298801c7972986d05f7e66d8 Mon Sep 17 00:00:00 2001 From: Drew Stone Date: Sat, 22 Aug 2026 11:26:40 -0700 Subject: [PATCH 1/2] fix(deps): widen agent-eval peer range The peer range `>=0.163.2 <0.164.0` admitted exactly one published Eval version. Eval `latest` is 0.170.0, so a consumer that installed the cohort got an unmet peer on agent-knowledge and could not complete the install. Move the Eval peer range to `>=0.170.0 <0.171.0` and the Interface peer range to `^1.6.0`, and move both development pins with them. A pre-1.0 dependency earns a single-minor range, which `scripts/lib/peer-range.mjs` derives from the development pin, so the range tracks the version the suite runs against. Interface moves because Eval 0.170.0 depends on agent-core 0.9.5, which requires Interface `^1.5.0`. Holding Interface at 1.4.0 leaves a second physical copy of a contract package in the tree. The ceiling is measured. This package imports 79 distinct symbols from Eval across `.`, `/campaign`, `/rl`, `/experiment`, and `/analyst`, plus one dynamic import of `/campaign`. A compiler probe over every symbol gives the same result against 0.163.2 and 0.170.0. The six `/analyst` exports Eval 0.170.0 removes are not imported here. All 15 Interface symbols resolve at 1.6.0 as they do at 1.4.0. The version moves to 10.7.1 because `check:version-bump` requires a consumer-visible manifest change to ship under a higher version. Publishing stays gated on a `v*` tag push. Closes #174 --- CHANGELOG.md | 19 +++++++++++++++++++ package.json | 10 +++++----- pnpm-lock.yaml | 24 ++++++++++++------------ 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f02a0db..242cd05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,24 @@ # Changelog +## 10.7.1 — 2026-08-22 + +### Changed + +- Accept Eval `>=0.170.0 <0.171.0`, replacing Eval `>=0.163.2 <0.164.0`, and Interface `^1.6.0`, replacing Interface `^1.4.0`. +The old Eval range admitted exactly one published version while Eval `latest` was 0.170.0, so a consumer installing the cohort got an unmet peer on this package and could not complete the install. +- **A consumer must move Eval and Interface with this package.** +Interface moves because Eval 0.170.0 depends on agent-core 0.9.5, which requires Interface `^1.5.0`. +Interface 1.4.0 leaves a second physical copy of the contract package in the tree, and one contract package must resolve to one copy. +Move Eval to 0.170.x and Interface to 1.6.x in the same change. +- The new Eval ceiling is measured, not assumed. +This package imports 79 distinct symbols from Eval across five entry points: `.` (37), `/campaign` (35), `/rl` (4), `/experiment` (2), and `/analyst` (1), plus one dynamic `import()` of `/campaign`. +A compiler probe over every symbol gives the same result against 0.163.2 and against 0.170.0: 78 resolve, and `JsonValue` resolves in neither, because two test files import it from `/campaign` where it has never been exported. +Eval 0.170.0 removes six `/analyst` exports, which are `createJudgeAdapter`, `createRunCriticAdapter`, `createVerifierAdapter`, and their three option types. +This package imports none of the six. +All 15 symbols this package imports from Interface resolve at 1.6.0 exactly as they do at 1.4.0. +- A pre-1.0 dependency earns a single-minor range, so `<0.171.0` is the boundary the evidence covers. +An Eval minor above 0.170.x must be verified before the range admits it. + ## 10.7.0 — 2026-08-21 ### Added diff --git a/package.json b/package.json index 441d899..b12fb0a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@tangle-network/agent-knowledge", - "version": "10.7.0", + "version": "10.7.1", "description": "Build, search, evaluate, and improve source-backed knowledge bases.", "homepage": "https://github.com/tangle-network/agent-knowledge#readme", "repository": { @@ -83,15 +83,15 @@ "zod": "^4.4.3" }, "peerDependencies": { - "@tangle-network/agent-eval": ">=0.163.2 <0.164.0", - "@tangle-network/agent-interface": "^1.4.0" + "@tangle-network/agent-eval": ">=0.170.0 <0.171.0", + "@tangle-network/agent-interface": "^1.6.0" }, "devDependencies": { "@arethetypeswrong/cli": "^0.18.5", "@biomejs/biome": "^2.5.5", "@neo4j-labs/agent-memory": "0.4.1", - "@tangle-network/agent-eval": "0.163.2", - "@tangle-network/agent-interface": "1.4.0", + "@tangle-network/agent-eval": "0.170.0", + "@tangle-network/agent-interface": "1.6.0", "@types/node": "^26.1.1", "mem0ai": "3.1.2", "oxc-parser": "0.144.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3b77750..fdcbc21 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -35,11 +35,11 @@ importers: specifier: 0.4.1 version: 0.4.1 '@tangle-network/agent-eval': - specifier: 0.163.2 - version: 0.163.2 + specifier: 0.170.0 + version: 0.170.0 '@tangle-network/agent-interface': - specifier: 1.4.0 - version: 1.4.0 + specifier: 1.6.0 + version: 1.6.0 '@types/node': specifier: ^26.1.1 version: 26.1.1 @@ -763,13 +763,13 @@ packages: '@modelcontextprotocol/sdk': optional: true - '@tangle-network/agent-eval@0.163.2': - resolution: {integrity: sha512-uKhaHpeSgwiExr9fhU4aHuhK0bb5U7bZEf4uH2ElA+LyJIhXA7suMBPCHWVeIhiun9Nz8hbemOxX60Z1J5W0YQ==} + '@tangle-network/agent-eval@0.170.0': + resolution: {integrity: sha512-GoUIrtnXbN4gSDxHNB3c8vhA2JvPTlLtx7dnWZRCUFArQU2EMIvqhyVPaKnCmoJgDfbPgCwRuv60Pw2w9jSOeA==} engines: {node: '>=20'} hasBin: true - '@tangle-network/agent-interface@1.4.0': - resolution: {integrity: sha512-oUN5Ok0DJuNXIwlpi/ZHLeOUQq/LSAAa1Boe4RTt7S4feAWGddya1fTgQogHbnRiHEswFHdw2wvpEb8qyVx49g==} + '@tangle-network/agent-interface@1.6.0': + resolution: {integrity: sha512-DzdRAOTuPacezJlvOo+Xu5fWdoE9sJg90CDDo+SOCUJmfxrg38bEgCQz8ujr4r0Z5ABG2DlazMFRG/1/XoQ0HQ==} '@tangle-network/agent-trace-contract@1.0.2': resolution: {integrity: sha512-v7uMh56jkEp4vckevEU9xKsIatbs5dqzGPp69dFLSSXUVit0RP6VD6EANMXVlTCUk+6wVKBLHJx23XspVCEiIA==} @@ -2981,15 +2981,15 @@ snapshots: '@tangle-network/agent-core@0.9.4': dependencies: - '@tangle-network/agent-interface': 1.4.0 + '@tangle-network/agent-interface': 1.6.0 zod: 4.4.3 - '@tangle-network/agent-eval@0.163.2': + '@tangle-network/agent-eval@0.170.0': dependencies: '@asteasolutions/zod-to-openapi': 9.1.0(zod@4.4.3) '@hono/node-server': 2.0.12(hono@4.12.32) '@tangle-network/agent-core': 0.9.4 - '@tangle-network/agent-interface': 1.4.0 + '@tangle-network/agent-interface': 1.6.0 '@tangle-network/agent-trace-contract': 1.0.2 hono: 4.12.32 linear-sum-assignment: 1.0.9 @@ -2998,7 +2998,7 @@ snapshots: transitivePeerDependencies: - '@modelcontextprotocol/sdk' - '@tangle-network/agent-interface@1.4.0': + '@tangle-network/agent-interface@1.6.0': dependencies: '@noble/hashes': 1.8.0 spdx-expression-parse: 5.0.0 From a9aa833b07ba648457f6f0a0e0cc72f96a87c66b Mon Sep 17 00:00:00 2001 From: Drew Stone Date: Sat, 22 Aug 2026 11:56:05 -0700 Subject: [PATCH 2/2] fix(file-transaction): compare a transaction root and its knowledge root in canonical form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `withTransactionRoot` tested containment with `resolve` on both sides. `resolve` is lexical: it cannot read a symbolic link. The transaction root reaches that test already canonical, because `openSafeDirectoryTree` canonicalizes the root it opens and `withSafeDirectory` hands the result back, while the knowledge root beside it carries whatever spelling the caller supplied. One directory arrived under two strings and the containment test rejected a directory that was inside its root. macOS reaches this state unaided: `os.tmpdir()` is `/var/folders/...`, a link to `/private/var/folders/...`. Any platform reaches it when a caller passes a root through a symbolic link. Canonicalize both sides before comparing, which is what `openSafeDirectoryTree` already does with the root it opens. Canonicalizing both sides also tightens the boundary: a candidate that leaves the root through a link is now rejected, where the lexical comparison admitted it. Give path comparison one owner in `src/durable-fs.ts` — `relativeWithinRoot`, `canonicalRelativeWithinRoot`, and `canonicalPathsEqual` — and route the four other sites through it, including two that compared a persisted root against a supplied root. Add `check:path-containment`, which fails a path comparison written inline. On Linux `/tmp` is a real directory, so both spellings coincide and the suite that gates the merge cannot see this class of defect; the gate reads the source instead. It reports 5 findings against the parent commit and none after. The regression test builds the asymmetry with an explicit symbolic link rather than relying on the temporary directory, so it fails on Linux too. Refs #174 --- CHANGELOG.md | 21 ++++ api-surface.json | 3 + package.json | 5 +- scripts/check-path-containment.mjs | 155 +++++++++++++++++++++++++++++ src/durable-fs.ts | 86 +++++++++++++++- src/file-transaction.ts | 15 +-- src/kb-improvement/state.ts | 19 ++-- src/kb-improvement/transition.ts | 2 +- src/kb-improvement/workspace.ts | 11 +- tests/file-transaction.test.ts | 23 +++++ 10 files changed, 308 insertions(+), 32 deletions(-) create mode 100644 scripts/check-path-containment.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 242cd05..428712c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,26 @@ # Changelog +## 10.8.0 — 2026-08-22 + +### Fixed + +- A knowledge root that canonicalizes to a different path than the caller holds now opens its file transactions instead of failing with `knowledge transaction directory escaped its root`. +`withTransactionRoot` compared the transaction root against the knowledge root with `resolve` on both sides. +`resolve` is lexical and cannot read a symbolic link, while the transaction root arrives already canonical from `withSafeDirectory`, so the two described one directory with two strings and the containment test rejected a directory that was inside the root. +On macOS this happened on every run that placed a knowledge base under `os.tmpdir()`, because `/var/folders/...` is a link to `/private/var/folders/...`. +It also happened on any platform when a caller passed a root reached through a symbolic link. +- `loadRunState` and `assertStateIdentity` compared a persisted root against a supplied root the same way, and reported a mismatch between two spellings of one directory. + +### Added + +- `relativeWithinRoot(root, candidate)`, `canonicalRelativeWithinRoot(root, candidate)`, and `canonicalPathsEqual(left, right)`. +`src/durable-fs.ts` now owns every comparison between two filesystem paths. +`canonicalRelativeWithinRoot` and `canonicalPathsEqual` canonicalize both sides first, and neither path has to exist: the deepest existing ancestor is canonicalized and the remaining segments are appended, so a directory that is about to be created is measured against the same root as one that already is. +Canonicalizing both sides also tightens the boundary, because a candidate that leaves the root through a symbolic link is rejected where a lexical comparison admits it. +- `pnpm run check:path-containment`, which fails a path comparison written inline instead of through those owners. +It runs inside `verify:package`, so both workflows enforce it. +A lexical comparison looks correct on Linux, where `/tmp` is a real directory and the two forms coincide, so this class of defect cannot be caught by running the suite on the machine that gates the merge. + ## 10.7.1 — 2026-08-22 ### Changed diff --git a/api-surface.json b/api-surface.json index 5ced263..77a4723 100644 --- a/api-surface.json +++ b/api-surface.json @@ -547,6 +547,8 @@ "buildRetrievalBenchmarkCasesFromQrels": "value 0cee488c25c6", "buildRetrievalEvalDispatch": "value 4f12d1f4c065", "calibrateRagAnswerJudge": "value 74f91dafd1b1", + "canonicalPathsEqual": "value 1a1d22b3b1c3", + "canonicalRelativeWithinRoot": "value 9a5ffa3946f3", "canonicalizeUrl": "value a8a18cde9fcc", "chunkMarkdown": "value 3f930d4ba32f", "citedClaimKey": "value 810f529bf94c", @@ -692,6 +694,7 @@ "readRegularFileWithinRoot": "value 310c63c14033", "reciprocalRankFusion": "value 63d0b7d7082b", "recoverPendingKnowledgeMutation": "value 658b76e5b390", + "relativeWithinRoot": "value bd8a9ab05814", "removeDurable": "value 6218521600a7", "renameDurable": "value ea60b9d3f720", "renderKnowledgeBenchmarkReportMarkdown": "value 54198213283b", diff --git a/package.json b/package.json index b12fb0a..830bbf0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@tangle-network/agent-knowledge", - "version": "10.7.1", + "version": "10.8.0", "description": "Build, search, evaluate, and improve source-backed knowledge bases.", "homepage": "https://github.com/tangle-network/agent-knowledge#readme", "repository": { @@ -71,7 +71,8 @@ "lint": "biome check src tests", "format": "biome format --write src tests", "check:skills": "node scripts/check-skills.mjs", - "verify:package": "pnpm run check:version-bump && pnpm run check:skills && pnpm run check:api-surface && publint && attw --pack --profile esm-only . && node scripts/verify-package.mjs", + "check:path-containment": "node scripts/check-path-containment.mjs", + "verify:package": "pnpm run check:version-bump && pnpm run check:skills && pnpm run check:path-containment && pnpm run check:api-surface && publint && attw --pack --profile esm-only . && node scripts/verify-package.mjs", "verify:official-optimizers": "node scripts/verify-official-optimizers.mjs", "check:version-bump": "node scripts/check-version-bump.mjs", "check:api-surface": "node scripts/check-api-surface.mjs", diff --git a/scripts/check-path-containment.mjs b/scripts/check-path-containment.mjs new file mode 100644 index 0000000..2957031 --- /dev/null +++ b/scripts/check-path-containment.mjs @@ -0,0 +1,155 @@ +#!/usr/bin/env node +/** + * Fail a path comparison that does not canonicalize the paths it compares. + * + * `resolve` is lexical. It expands `.` and `..` and makes a path absolute, and + * it stops there: it cannot read a symbolic link. Two strings that name one + * directory therefore compare as different whenever one of them has been + * through `realpath` and the other has not. + * + * That state is not rare in this package. `openSafeDirectoryTree` canonicalizes + * the root it opens, so every path a caller receives through `withSafeDirectory` + * is canonical, while a path the same caller builds with `join` is not. On macOS + * the operating system supplies the asymmetry unaided: `os.tmpdir()` is + * `/var/folders/...`, a link to `/private/var/folders/...`. + * + * The failure is silent in CI. Linux `/tmp` is a real directory, so the two + * forms coincide and a lexical comparison looks correct on the machine that + * gates the merge, while it rejects a directory that is genuinely inside the + * root on the machine that runs the code. + * + * So path comparison has one owner, `src/durable-fs.ts`, which canonicalizes + * both sides before it compares them: + * + * - `relativeWithinRoot(root, candidate)` — lexical, for two paths that + * already share one basis. + * - `canonicalRelativeWithinRoot(root, candidate)` — canonicalizes both sides. + * - `canonicalPathsEqual(left, right)` — the same, for equality. + * + * This check rejects the three shapes that reimplement one of those inline. + * + * Usage: pnpm run check:path-containment + */ +import { readdirSync, readFileSync, statSync } from 'node:fs' +import { join, relative, resolve } from 'node:path' +import { fileURLToPath } from 'node:url' +import { parseSync } from 'oxc-parser' + +const repoRoot = resolve(fileURLToPath(new URL('../', import.meta.url))) +const sourceRoot = join(repoRoot, 'src') + +/** The one module allowed to compare two filesystem paths. */ +const owner = join(sourceRoot, 'durable-fs.ts') + +const files = [] +const collect = (directory) => { + for (const entry of readdirSync(directory)) { + const path = join(directory, entry) + if (statSync(path).isDirectory()) { + collect(path) + } else if (entry.endsWith('.ts') && !entry.endsWith('.test.ts')) { + files.push(path) + } + } +} +collect(sourceRoot) + +const findings = [] + +function walk(node, visit) { + if (node === null || typeof node !== 'object') return + if (Array.isArray(node)) { + for (const item of node) walk(item, visit) + return + } + if (typeof node.type === 'string') visit(node) + for (const key of Object.keys(node)) { + if (key === 'type') continue + walk(node[key], visit) + } +} + +const isCallTo = (node, name) => + node?.type === 'CallExpression' && + node.callee?.type === 'Identifier' && + node.callee.name === name + +const isStringLiteral = (node, value) => + node?.type === 'Literal' && node.value === value + +/** A template that interpolates the path separator builds a prefix to compare against. */ +const interpolatesSeparator = (node) => + node?.type === 'TemplateLiteral' && + node.expressions.some( + (expression) => expression.type === 'Identifier' && expression.name === 'sep', + ) + +for (const file of files) { + if (file === owner) continue + const source = readFileSync(file, 'utf8') + const parsed = parseSync(file, source, { lang: 'ts' }) + if (parsed.errors.length > 0) { + throw new Error(`${file} failed to parse: ${parsed.errors[0].message}`) + } + const where = (node) => `${relative(repoRoot, file)}:${source.slice(0, node.start).split('\n').length}` + + walk(parsed.program, (node) => { + // `candidate.startsWith(`${root}${sep}`)` — a prefix test standing in for containment. + if ( + node.type === 'CallExpression' && + node.callee?.type === 'MemberExpression' && + node.callee.property?.name === 'startsWith' && + node.arguments.length === 1 + ) { + if (interpolatesSeparator(node.arguments[0])) { + findings.push( + `${where(node)}: a path prefix test built from \`sep\`. Call relativeWithinRoot or canonicalRelativeWithinRoot from durable-fs instead.`, + ) + } + if (isStringLiteral(node.arguments[0], '../')) { + findings.push( + `${where(node)}: an inline escape test on a relative path. Call relativeWithinRoot or canonicalRelativeWithinRoot from durable-fs instead.`, + ) + } + } + + // `relative(a, b) === '..'` — the same escape test written as a comparison. + if ( + node.type === 'BinaryExpression' && + ['===', '!==', '==', '!='].includes(node.operator) && + (isStringLiteral(node.right, '..') || isStringLiteral(node.left, '..')) + ) { + const other = isStringLiteral(node.right, '..') ? node.left : node.right + if (isCallTo(other, 'relative')) { + findings.push( + `${where(node)}: an inline escape test on relative(). Call relativeWithinRoot or canonicalRelativeWithinRoot from durable-fs instead.`, + ) + } + } + + // `resolve(a) === resolve(b)` — two paths compared without canonicalizing either. + if ( + node.type === 'BinaryExpression' && + ['===', '!==', '==', '!='].includes(node.operator) && + isCallTo(node.left, 'resolve') && + isCallTo(node.right, 'resolve') + ) { + findings.push( + `${where(node)}: two resolved paths compared for equality. Call canonicalPathsEqual from durable-fs instead.`, + ) + } + }) +} + +if (findings.length > 0) { + console.error('A path comparison must canonicalize both sides.\n') + for (const finding of findings) console.error(` ${finding}`) + console.error( + `\n${findings.length} finding(s). src/durable-fs.ts owns path comparison; see the header of scripts/check-path-containment.mjs.`, + ) + process.exit(1) +} + +console.log( + `Path comparison is owned by src/durable-fs.ts: ${files.length} source file(s) carry no inline path comparison.`, +) diff --git a/src/durable-fs.ts b/src/durable-fs.ts index 16a4804..ee3aa1c 100644 --- a/src/durable-fs.ts +++ b/src/durable-fs.ts @@ -10,7 +10,7 @@ import { rename, rm, } from 'node:fs/promises' -import { basename, dirname, isAbsolute, resolve } from 'node:path' +import { basename, dirname, isAbsolute, join, relative, resolve } from 'node:path' const DIRECTORY_FLAGS = constants.O_RDONLY | constants.O_DIRECTORY | constants.O_NOFOLLOW const CREATE_FILE_FLAGS = @@ -290,6 +290,90 @@ export function isKernelAnchoredPath(path: string): boolean { return process.platform === 'linux' && /^\/proc\/self\/fd\/\d+(?:\/|$)/.test(path) } +/** + * The path from `root` down to `candidate`, or undefined when `candidate` is not + * strictly inside `root`. + * + * The comparison is lexical, so both arguments must already share one + * canonicalization basis. Use `canonicalRelativeWithinRoot` when either side can + * carry a different basis. + * + * An empty result means the two paths are equal. A root is not inside itself, + * and a caller that treats it as a descendant would hand its own root to + * machinery that may remove the directory it is given. + */ +export function relativeWithinRoot(root: string, candidate: string): string | undefined { + const value = relative(resolve(root), resolve(candidate)).replace(/\\/g, '/') + if (value === '' || value === '..' || value.startsWith('../') || isAbsolute(value)) { + return undefined + } + return value +} + +/** + * The path from `root` down to `candidate`, with both sides canonicalized before + * they are compared. + * + * `resolve` is lexical: it cannot read a symbolic link, so a path that has been + * through `realpath` and one that has not describe the same directory with + * different strings, and comparing the two rejects a directory that genuinely + * sits inside the root. `openSafeDirectoryTree` canonicalizes the root it opens, + * so any path that reaches a caller through `withSafeDirectory` carries the + * canonical form while a path the caller built itself does not. + * + * Canonicalizing both sides also tightens the boundary: a candidate that leaves + * the root through a symbolic link is rejected here, where a lexical comparison + * admits it. + * + * Neither path has to exist. The deepest existing ancestor of each is + * canonicalized and the remaining segments are appended, so a directory that is + * about to be created is measured against the same root as one that already is. + */ +export async function canonicalRelativeWithinRoot( + root: string, + candidate: string, +): Promise { + const canonicalRoot = isKernelAnchoredPath(root) ? root : await canonicalizeThroughMissing(root) + const canonicalCandidate = isKernelAnchoredPath(candidate) + ? candidate + : await canonicalizeThroughMissing(candidate) + return relativeWithinRoot(canonicalRoot, canonicalCandidate) +} + +/** + * Whether two paths name the same location, with both sides canonicalized first. + * + * A path stored by one caller and a path supplied by another reach this + * comparison through different routes, so the same location can arrive under two + * spellings. Comparing the resolved strings reports a mismatch that does not + * exist. + */ +export async function canonicalPathsEqual(left: string, right: string): Promise { + const [canonicalLeft, canonicalRight] = await Promise.all([ + isKernelAnchoredPath(left) ? left : canonicalizeThroughMissing(left), + isKernelAnchoredPath(right) ? right : canonicalizeThroughMissing(right), + ]) + return canonicalLeft === canonicalRight +} + +/** The canonical form of a path whose trailing segments may not exist yet. */ +async function canonicalizeThroughMissing(path: string): Promise { + const missing: string[] = [] + let current = resolve(path) + for (;;) { + try { + const existing = await realpath(current) + return missing.length === 0 ? existing : join(existing, ...missing.reverse()) + } catch (error) { + if (!isMissingFile(error)) throw error + const parent = dirname(current) + if (parent === current) throw error + missing.push(basename(current)) + current = parent + } + } +} + function anchoredDirectoryPath(handle: FileHandle, fallback: string): string { return process.platform === 'linux' ? `/proc/self/fd/${handle.fd}` : fallback } diff --git a/src/file-transaction.ts b/src/file-transaction.ts index f100291..cac62ac 100644 --- a/src/file-transaction.ts +++ b/src/file-transaction.ts @@ -1,9 +1,10 @@ import { createHash, randomUUID } from 'node:crypto' import { mkdtemp, readdir, rm } from 'node:fs/promises' -import { join, relative, resolve, sep } from 'node:path' +import { join } from 'node:path' import { contentHash } from '@tangle-network/agent-eval' import { z } from 'zod' import { + canonicalRelativeWithinRoot, isKernelAnchoredPath, isMissingFile, readRegularFileNoFollow, @@ -617,17 +618,11 @@ async function withTransactionRoot( if (!match?.[1]) throw new Error('knowledge transaction directory has an invalid anchor') return withSafeDirectory(match[1], match[2] ?? '', create, use) } - const resolvedRoot = resolve(root) - const resolvedTransactionRoot = resolve(transactionRoot) - if (!resolvedTransactionRoot.startsWith(`${resolvedRoot}${sep}`)) { + const within = await canonicalRelativeWithinRoot(root, transactionRoot) + if (within === undefined) { throw new Error('knowledge transaction directory escaped its root') } - return withSafeDirectory( - root, - relative(resolvedRoot, resolvedTransactionRoot).replace(/\\/g, '/'), - create, - use, - ) + return withSafeDirectory(root, within, create, use) } function assertTransactionEntries(transaction: KnowledgeFileTransaction): void { diff --git a/src/kb-improvement/state.ts b/src/kb-improvement/state.ts index ac340f6..e57b96b 100644 --- a/src/kb-improvement/state.ts +++ b/src/kb-improvement/state.ts @@ -1,10 +1,12 @@ import { stat } from 'node:fs/promises' -import { isAbsolute, join, relative, resolve, sep } from 'node:path' +import { join } from 'node:path' import { canonicalJson, contentHash } from '@tangle-network/agent-eval' import { + canonicalPathsEqual, isMissingFile, listRegularFilesWithinRoot, readRegularFileWithinRoot, + relativeWithinRoot, withSafeDirectory, writeJsonDurableWithinRoot, } from '../durable-fs' @@ -31,9 +33,7 @@ export function knowledgeImprovementRunDir(root: string, runId: string): string : `${slugify(parsedRunId).slice(0, 72)}-${sha256(parsedRunId).slice(0, 16)}` const improvementsDir = join(layoutFor(root).cacheDir, 'improvements') const runDir = join(improvementsDir, runSegment) - const resolvedImprovementsDir = resolve(improvementsDir) - const resolvedRunDir = resolve(runDir) - if (!resolvedRunDir.startsWith(`${resolvedImprovementsDir}${sep}`)) { + if (relativeWithinRoot(improvementsDir, runDir) === undefined) { throw new Error('knowledge improvement run directory escaped its root') } return runDir @@ -46,7 +46,7 @@ export async function withKnowledgeImprovementRun( use: (runDir: string) => Promise | T, ): Promise { const runDir = knowledgeImprovementRunDir(root, runId) - const relativePath = descendantPath(root, runDir) + const relativePath = relativeWithinRoot(root, runDir) if (!relativePath) throw new Error('knowledge improvement run directory escaped its root') return withSafeDirectory(root, relativePath, create, async (openedRunDir) => { const result = await use(openedRunDir) @@ -87,7 +87,7 @@ export async function loadKnowledgeImprovementStateFromRun( if (state.runId !== runId) { throw new Error('knowledge improvement state does not match the requested run') } - if (resolve(state.root) !== resolve(root)) { + if (!(await canonicalPathsEqual(state.root, root))) { throw new Error('knowledge improvement state does not match the requested root') } for (const candidate of state.candidates) { @@ -252,13 +252,6 @@ export function candidateEvidenceRelativePath(candidateId: string): string { ) } -function descendantPath(root: string, path: string): string | undefined { - const value = relative(resolve(root), resolve(path)).replace(/\\/g, '/') - if (value === '' || value === '..' || value.startsWith('../') || isAbsolute(value)) - return undefined - return value -} - export function assertExactCandidatePlatform(): void { if (process.platform !== 'linux') { throw new Error('exact knowledge candidate workflows require Linux directory descriptors') diff --git a/src/kb-improvement/transition.ts b/src/kb-improvement/transition.ts index 6138d33..bcc49a8 100644 --- a/src/kb-improvement/transition.ts +++ b/src/kb-improvement/transition.ts @@ -135,7 +135,7 @@ async function applyKnowledgeCandidateTarget( target: KnowledgeImprovementTarget, ): Promise { const { candidateRef, runDir, state } = input - assertStateIdentity(input.root, candidateRef, state) + await assertStateIdentity(input.root, candidateRef, state) const candidate = state.candidates.find((entry) => entry.candidateId === candidateRef.candidateId) if ( !candidate || diff --git a/src/kb-improvement/workspace.ts b/src/kb-improvement/workspace.ts index 5e261a0..2cadd24 100644 --- a/src/kb-improvement/workspace.ts +++ b/src/kb-improvement/workspace.ts @@ -1,9 +1,10 @@ import { createHash } from 'node:crypto' import { cp, lstat, mkdir, mkdtemp, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' -import { dirname, join, relative, resolve } from 'node:path' +import { dirname, join, relative } from 'node:path' import { canonicalJson, contentHash } from '@tangle-network/agent-eval' import { + canonicalPathsEqual, isMissingFile, listRegularFilesWithinRoot, readRegularFileWithinRoot, @@ -155,7 +156,7 @@ export async function withMeasuredCandidateSnapshot( evidence: KnowledgeImprovementEvidence }) => Promise | T, ): Promise { - assertStateIdentity(liveRoot, candidateRef, state) + await assertStateIdentity(liveRoot, candidateRef, state) const candidate = state.candidates.find((entry) => entry.candidateId === candidateRef.candidateId) if (!candidate) { throw new Error(`knowledge candidate '${candidateRef.candidateId}' does not exist`) @@ -242,15 +243,15 @@ export async function assertCandidateEvidence( return evidence } -export function assertStateIdentity( +export async function assertStateIdentity( root: string, candidateRef: KnowledgeImprovementCandidateRef, state: KnowledgeImprovementRunState, -): void { +): Promise { if (state.runId !== candidateRef.runId) { throw new Error('knowledge candidate run identity does not match persisted state') } - if (resolve(state.root) !== resolve(root)) { + if (!(await canonicalPathsEqual(state.root, root))) { throw new Error('knowledge candidate root does not match persisted state') } if (sha256(state.goal) !== candidateRef.goalHash) { diff --git a/tests/file-transaction.test.ts b/tests/file-transaction.test.ts index ef06d49..5ac5193 100644 --- a/tests/file-transaction.test.ts +++ b/tests/file-transaction.test.ts @@ -41,6 +41,29 @@ async function withRoot(fn: (root: string) => Promise): Promise { } describe('knowledge file transactions', () => { + /** + * A root reached through a symbolic link canonicalizes to a different string + * than the one the caller holds, while the transaction root arrives already + * canonical from `withSafeDirectory`. A lexical comparison between the two + * rejects a directory that is inside the root. macOS reaches this state on + * every run through `/var` -> `/private/var`; the link here reproduces it on + * any platform, including one whose temporary directory is not linked. + */ + it('opens a transaction root under a symbolically linked knowledge root', async () => { + await withRoot(async (realRoot) => { + const linkParent = await mkdtemp(join(tmpdir(), 'agent-knowledge-linked-')) + const linkedRoot = join(linkParent, 'root') + await symlink(realRoot, linkedRoot, 'dir') + try { + await expect(withKnowledgeMutation(linkedRoot, async () => 'committed')).resolves.toBe( + 'committed', + ) + } finally { + await rm(linkParent, { recursive: true, force: true }) + } + }) + }) + it('resumes an interrupted multi-file commit without mixing versions', async () => { await withRoot(async (root) => { const transactionRoot = join(root, '.transactions')