Skip to content

Commit 4d7d9be

Browse files
authored
fix(host-kit): resolve the code-signature root so a symlinked checkout stamps its own files (#2429)
walkDaemonCodeGraph resolved manifests through realpath but left the root and the entry as given. Under a symlinked prefix — macOS /tmp, a symlinked checkout — the two then sat on either side of the link: every workspace package was labelled by the route out of the repository and back in (../../../private/tmp/...), and isInstalledDependencyPath read every installed dependency as a workspace package and walked its whole closure. Route the root, the entry, and the manifest through one resolver, and give the cache the same resolved pair so a document's entry label still matches the walk that wrote it. The cache's private copy of that resolver goes away. The fixtures now build a resolved root, which is the shape a production caller passes (findProjectRoot derives it from an already-resolved import.meta.url). Naming a root through a link is covered on purpose instead, by two tests that fail without this change on any platform.
1 parent 342e98c commit 4d7d9be

5 files changed

Lines changed: 139 additions & 55 deletions

File tree

packages/host-kit/src/code-signature-cache.test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@ import path from 'node:path';
55
import { afterEach, test, vi } from 'vitest';
66
import { computeDaemonCodeSignature } from './code-signature.ts';
77
import { resolveCachedDaemonCodeSignature } from './code-signature-cache.ts';
8-
import { writeWorkspaceFixture } from './code-signature.fixtures.ts';
9-
import { mkdtempForTestSync } from './internal/tmp-dir.fixtures.ts';
8+
import { createCheckoutRootForTest, writeWorkspaceFixture } from './code-signature.fixtures.ts';
109

1110
afterEach(() => {
1211
vi.restoreAllMocks();
@@ -38,7 +37,7 @@ function writeGraphFixture(
3837
depPath: string;
3938
cacheHome: string;
4039
} {
41-
const root = mkdtempForTestSync(prefix);
40+
const root = createCheckoutRootForTest(prefix);
4241
const entryPath = path.join(root, 'src', 'daemon.ts');
4342
const depPath = path.join(root, 'src', dependency.fileName);
4443
fs.mkdirSync(path.dirname(entryPath), { recursive: true });
@@ -314,7 +313,7 @@ test('resolveCachedDaemonCodeSignature ignores a cache document another user wro
314313
});
315314

316315
test('resolveCachedDaemonCodeSignature reports an unreadable entry as unknown', () => {
317-
const root = mkdtempForTestSync('agent-device-signature-cache-missing-');
316+
const root = createCheckoutRootForTest('agent-device-signature-cache-missing-');
318317
try {
319318
assert.equal(
320319
resolveCachedDaemonCodeSignature(path.join(root, 'src', 'daemon.ts'), root),

packages/host-kit/src/code-signature-cache.ts

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { publishFileSync } from './file.ts';
66
import {
77
buildDaemonCodeFileLabel,
88
formatDaemonCodeSignature,
9+
resolveDaemonCodePath,
910
walkDaemonCodeGraph,
1011
type DaemonCodeFileStamp,
1112
type DaemonCodeGraphWalk,
@@ -57,14 +58,18 @@ type CacheDocument = {
5758
* toolchain cache: failing to read or write one only costs the walk.
5859
*/
5960
export function resolveCachedDaemonCodeSignature(entryPath: string, root: string): string {
60-
const cachePath = resolveCachePath(entryPath, root);
61-
const entryLabel = buildDaemonCodeFileLabel(root, entryPath);
62-
const validated = readValidatedWalk(cachePath, root, entryLabel);
61+
// The walk names every file from the resolved root down, so a document is
62+
// read and written under those same names or its entry never matches.
63+
const realRoot = resolveDaemonCodePath(root);
64+
const realEntryPath = resolveDaemonCodePath(entryPath);
65+
const cachePath = resolveCachePath(realEntryPath, realRoot);
66+
const entryLabel = buildDaemonCodeFileLabel(realRoot, realEntryPath);
67+
const validated = readValidatedWalk(cachePath, realRoot, entryLabel);
6368
if (validated) return formatDaemonCodeSignature(validated.files);
6469

6570
let walk: DaemonCodeGraphWalk;
6671
try {
67-
walk = walkDaemonCodeGraph(entryPath, root);
72+
walk = walkDaemonCodeGraph(realEntryPath, realRoot);
6873
} catch {
6974
return 'unknown';
7075
}
@@ -195,21 +200,13 @@ function publishWalk(cachePath: string, walk: DaemonCodeGraphWalk): void {
195200
}
196201
}
197202

198-
/** One document per (entry, root) pair, so sibling checkouts never share one. */
203+
/**
204+
* One document per (entry, root) pair, so sibling checkouts never share one.
205+
* Both arrive resolved, so symlinked routes to one checkout are one checkout,
206+
* as in `buildSourceCheckoutStateDirName`.
207+
*/
199208
function resolveCachePath(entryPath: string, root: string): string {
200-
const key = crypto
201-
.createHash('sha1')
202-
.update(`${resolveRealPath(root)} ${resolveRealPath(entryPath)}`)
203-
.digest('hex');
209+
const key = crypto.createHash('sha1').update(`${root} ${entryPath}`).digest('hex');
204210
const directory = `${CACHE_DIRECTORY_PREFIX}-${process.getuid?.() ?? 'user'}`;
205211
return path.join(os.tmpdir(), directory, `${key}.json`);
206212
}
207-
208-
/** Symlinked paths to one checkout are one checkout, as in `buildSourceCheckoutStateDirName`. */
209-
function resolveRealPath(filePath: string): string {
210-
try {
211-
return fs.realpathSync.native(filePath);
212-
} catch {
213-
return path.resolve(filePath);
214-
}
215-
}

packages/host-kit/src/code-signature.fixtures.ts

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,18 @@ import fs from 'node:fs';
22
import path from 'node:path';
33
import { mkdtempForTestSync } from './internal/tmp-dir.fixtures.ts';
44

5+
/**
6+
* A temporary checkout root named the way a real one is: with its symlinks
7+
* resolved. A production root comes from `import.meta.url`, which the loader
8+
* resolved before the walk ever sees it. On macOS `os.tmpdir()` is `/tmp`,
9+
* itself a link to `/private/tmp`, so an unresolved fixture root would be a
10+
* shape no caller passes — and the shape every fixture passed. Naming a root
11+
* through a link is covered on purpose instead, by its own test.
12+
*/
13+
export function createCheckoutRootForTest(prefix: string): string {
14+
return fs.realpathSync.native(mkdtempForTestSync(prefix));
15+
}
16+
517
/**
618
* A source checkout that imports its own implementation the way this one does:
719
* an entry under `src/`, a workspace package under `packages/`, and the
@@ -16,7 +28,7 @@ export function writeWorkspaceFixture(prefix: string): {
1628
manifestPath: string;
1729
ownedPath: string;
1830
} {
19-
const root = mkdtempForTestSync(prefix);
31+
const root = createCheckoutRootForTest(prefix);
2032
const entryPath = path.join(root, 'src', 'daemon.ts');
2133
const packageDir = path.join(root, 'packages', 'kit');
2234
const ownedPath = path.join(packageDir, 'src', 'owned.ts');
@@ -40,3 +52,28 @@ export function writeWorkspaceFixture(prefix: string): {
4052
fs.symlinkSync(packageDir, path.join(linkDir, 'kit'), 'dir');
4153
return { root, entryPath, packageDir, manifestPath, ownedPath };
4254
}
55+
56+
/**
57+
* A checkout whose `@scope/vendor` is installed rather than linked: the
58+
* package directory is written inside `node_modules` in place, which is the
59+
* whole difference the walk keys the two cases on.
60+
*/
61+
export function writeInstalledDependencyFixture(prefix: string): {
62+
root: string;
63+
entryPath: string;
64+
} {
65+
const root = createCheckoutRootForTest(prefix);
66+
const entryPath = path.join(root, 'src', 'daemon.ts');
67+
const packageDir = path.join(root, 'node_modules', '@scope', 'vendor');
68+
69+
fs.mkdirSync(path.dirname(entryPath), { recursive: true });
70+
fs.mkdirSync(path.join(packageDir, 'src'), { recursive: true });
71+
fs.writeFileSync(entryPath, "import '@scope/vendor/thing';\n", 'utf8');
72+
fs.writeFileSync(path.join(packageDir, 'src', 'thing.js'), 'export const thing = 1;\n', 'utf8');
73+
fs.writeFileSync(
74+
path.join(packageDir, 'package.json'),
75+
JSON.stringify({ name: '@scope/vendor', exports: { './thing': './src/thing.js' } }),
76+
'utf8',
77+
);
78+
return { root, entryPath };
79+
}

packages/host-kit/src/code-signature.test.ts

Lines changed: 58 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,27 @@ import fs from 'node:fs';
33
import path from 'node:path';
44
import { test } from 'vitest';
55
import { computeDaemonCodeSignature, walkDaemonCodeGraph } from './code-signature.ts';
6-
import { writeWorkspaceFixture } from './code-signature.fixtures.ts';
7-
import { mkdtempForTestSync } from './internal/tmp-dir.fixtures.ts';
6+
import {
7+
createCheckoutRootForTest,
8+
writeInstalledDependencyFixture,
9+
writeWorkspaceFixture,
10+
} from './code-signature.fixtures.ts';
811

912
function labelsOf(entryPath: string, root: string): string[] {
1013
return walkDaemonCodeGraph(entryPath, root).files.map(([label]) => label);
1114
}
1215

16+
/**
17+
* The same checkout named through a symlink, which is how a real one is
18+
* routinely named: macOS resolves `/tmp` to `/private/tmp`, and a checkout
19+
* under a symlinked parent directory reaches every file the same way.
20+
*/
21+
function linkTo(root: string): string {
22+
const linkPath = `${root}-link`;
23+
fs.symlinkSync(root, linkPath, 'dir');
24+
return linkPath;
25+
}
26+
1327
test('a workspace subpath is walked, and its file is stamped under the package path', () => {
1428
const { root, entryPath } = writeWorkspaceFixture('agent-device-signature-workspace-');
1529
try {
@@ -60,28 +74,56 @@ test('retargeting the exports map moves the edge without either endpoint changin
6074
});
6175

6276
test('an installed dependency is not followed into', () => {
63-
const root = mkdtempForTestSync('agent-device-signature-installed-');
77+
const { root, entryPath } = writeInstalledDependencyFixture('agent-device-signature-installed-');
6478
try {
65-
const entryPath = path.join(root, 'src', 'daemon.ts');
66-
const packageDir = path.join(root, 'node_modules', '@scope', 'vendor');
67-
fs.mkdirSync(path.dirname(entryPath), { recursive: true });
68-
fs.mkdirSync(path.join(packageDir, 'src'), { recursive: true });
69-
fs.writeFileSync(entryPath, "import '@scope/vendor/thing';\n", 'utf8');
70-
fs.writeFileSync(path.join(packageDir, 'src', 'thing.js'), 'export const thing = 1;\n', 'utf8');
71-
fs.writeFileSync(
72-
path.join(packageDir, 'package.json'),
73-
JSON.stringify({ name: '@scope/vendor', exports: { './thing': './src/thing.js' } }),
74-
'utf8',
75-
);
76-
7779
assert.deepEqual(labelsOf(entryPath, root), ['src/daemon.ts']);
7880
} finally {
7981
fs.rmSync(root, { recursive: true, force: true });
8082
}
8183
});
8284

85+
/**
86+
* A root named through a symlink is the same root, and every label is still
87+
* repository-relative. Resolving the manifests but not the root would put the
88+
* two on either side of the link, so a workspace package would be labelled by
89+
* the route out of the repository and back in.
90+
*/
91+
test('a checkout named through a symlink stamps the same repository-relative labels', () => {
92+
const { root } = writeWorkspaceFixture('agent-device-signature-linked-workspace-');
93+
const linkedRoot = linkTo(root);
94+
try {
95+
assert.deepEqual(labelsOf(path.join(linkedRoot, 'src', 'daemon.ts'), linkedRoot).sort(), [
96+
'packages/kit/package.json',
97+
'packages/kit/src/owned.ts',
98+
'src/daemon.ts',
99+
]);
100+
} finally {
101+
fs.rmSync(linkedRoot, { force: true });
102+
fs.rmSync(root, { recursive: true, force: true });
103+
}
104+
});
105+
106+
/**
107+
* The workspace/installed test is `node_modules` containment, so it answers
108+
* only while the root and the manifest are named the same way. Under an
109+
* unresolved root every installed dependency reads as a workspace package and
110+
* the walk follows its whole closure.
111+
*/
112+
test('an installed dependency is not followed into through a symlinked root', () => {
113+
const { root } = writeInstalledDependencyFixture('agent-device-signature-linked-installed-');
114+
const linkedRoot = linkTo(root);
115+
try {
116+
assert.deepEqual(labelsOf(path.join(linkedRoot, 'src', 'daemon.ts'), linkedRoot), [
117+
'src/daemon.ts',
118+
]);
119+
} finally {
120+
fs.rmSync(linkedRoot, { force: true });
121+
fs.rmSync(root, { recursive: true, force: true });
122+
}
123+
});
124+
83125
test('a workspace package linked after the walk is an absent path, not a silent miss', () => {
84-
const root = mkdtempForTestSync('agent-device-signature-absent-');
126+
const root = createCheckoutRootForTest('agent-device-signature-absent-');
85127
try {
86128
const entryPath = path.join(root, 'src', 'daemon.ts');
87129
fs.mkdirSync(path.dirname(entryPath), { recursive: true });

packages/host-kit/src/code-signature.ts

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,8 @@ export function computeDaemonCodeSignature(
8787
* format guard relies on being inside the graph it walks.
8888
*/
8989
export function walkDaemonCodeGraph(entryPath: string, root: string): DaemonCodeGraphWalk {
90-
const normalizedRoot = path.resolve(root);
91-
const queue = [path.resolve(entryPath)];
90+
const normalizedRoot = resolveDaemonCodePath(root);
91+
const queue = [resolveDaemonCodePath(entryPath)];
9292
const visited = new Set<string>();
9393
const files: DaemonCodeFileStamp[] = [];
9494
const absentPaths = new Set<string>();
@@ -176,6 +176,26 @@ export function buildDaemonCodeFileLabel(root: string, filePath: string): string
176176
return path.relative(path.resolve(root), resolvedPath) || resolvedPath;
177177
}
178178

179+
/**
180+
* A path with its symlinks resolved, falling back to a plain resolve for a
181+
* path that names nothing yet.
182+
*
183+
* Every root, entry, and manifest a walk compares or labels goes through here,
184+
* so all of them name a file the same way. Two spellings of one path is not a
185+
* cosmetic difference: under a symlinked prefix — macOS `/tmp`, a symlinked
186+
* checkout — a root left unresolved sits outside the resolved tree beneath it,
187+
* so `buildDaemonCodeFileLabel` walks back out of the repository instead of
188+
* naming `packages/kit/...` and `isInstalledDependencyPath` reads every
189+
* installed dependency as a workspace package and walks its whole closure.
190+
*/
191+
export function resolveDaemonCodePath(filePath: string): string {
192+
try {
193+
return fs.realpathSync.native(filePath);
194+
} catch {
195+
return path.resolve(filePath);
196+
}
197+
}
198+
179199
/** The wire form of a signature; identical for a walked and a cache-validated stamp list. */
180200
export function formatDaemonCodeSignature(stamps: readonly DaemonCodeFileStamp[]): string {
181201
const fingerprint = stamps
@@ -244,7 +264,9 @@ function readWorkspacePackage(
244264
absentPaths.add(buildDaemonCodeFileLabel(root, linkedManifestPath));
245265
return null;
246266
}
247-
const manifestPath = realManifestPath(linkedManifestPath);
267+
// Both routes to the manifest name the same inode, so stamping it under the
268+
// package's own path keeps one label per file rather than one per route.
269+
const manifestPath = resolveDaemonCodePath(linkedManifestPath);
248270
if (isInstalledDependencyPath(root, manifestPath)) return null;
249271
try {
250272
return { manifestPath, manifest: JSON.parse(fs.readFileSync(manifestPath, 'utf8')) };
@@ -253,19 +275,6 @@ function readWorkspacePackage(
253275
}
254276
}
255277

256-
/**
257-
* The manifest's own path, so a package reached through its workspace link is
258-
* stamped under one label. Both routes name the same inode, so either would
259-
* revalidate; two labels for one file would just inflate every document.
260-
*/
261-
function realManifestPath(manifestPath: string): string {
262-
try {
263-
return fs.realpathSync.native(manifestPath);
264-
} catch {
265-
return manifestPath;
266-
}
267-
}
268-
269278
function isInstalledDependencyPath(root: string, filePath: string): boolean {
270279
const relative = path.relative(path.join(root, 'node_modules'), filePath);
271280
return relative !== '' && !relative.startsWith('..') && !path.isAbsolute(relative);

0 commit comments

Comments
 (0)