diff --git a/packages/agent-memory-sync/tests/integration/cross-machine-profiles.test.ts b/packages/agent-memory-sync/tests/integration/cross-machine-profiles.test.ts index 285e785..ec2571e 100644 --- a/packages/agent-memory-sync/tests/integration/cross-machine-profiles.test.ts +++ b/packages/agent-memory-sync/tests/integration/cross-machine-profiles.test.ts @@ -9,12 +9,70 @@ // flat .md files, so even a same-tree push would have missed most content. // // This test loads the ACTUAL committed profile files (profiles/macbook.json, -// profiles/mac-mini.json) via --config, overriding only what a hermetic test -// must override to avoid touching real machines or the network -// (--root-dir, --state-dir, --remote) — repositorySubdir and syncPaths come -// from the real files, unmodified, so a regression in either (e.g. -// reintroducing a per-machine repositorySubdir, or narrowing syncPaths back -// to named files/dirs) fails this test. +// profiles/mac-mini.json), so a regression in repositorySubdir (e.g. +// reintroducing a per-machine value) or in syncPaths shape (e.g. narrowing +// back to named files/dirs, dropping an entry, changing 'kind'/'ownerScoped') +// fails this test — everything about every syncPaths entry is taken from the +// real files verbatim EXCEPT one field on the two entries whose 'source' is a +// hardcoded machine-absolute path (machine-state, frictions; e.g. +// mac-mini.json's "/Users/lannguyensi/.harness/machine-state"). +// +// Those two sources get remapped (remapAbsoluteSyncPathsIntoSandbox below) +// into per-machine fake-$HOME directories under this test's own sandbox +// before the CLI ever runs, and assertSyncPathSourcesWithinSandbox asserts +// every syncPaths source — absolute sources directly, the "." memory +// entry's relative source resolved against the sandboxed rootDir — actually +// lands under the sandbox root before any sync executes. Without this, the +// test is not hermetic: it silently reads/writes whatever +// machine-state/frictions.json content happens to exist under the REAL +// path baked into the committed profile (agent-tasks 112a0864) — on a +// machine whose home genuinely is /Users/lannguyensi (e.g. the mini +// itself), that is live operational content (measured: an unmodified run +// of this file copies the real ~/.harness/machine-state/mac-mini.json and +// ~/.harness/frictions/mac-mini.json verbatim into an uncleaned bare git +// repo under the OS tmpdir). On a machine whose home is something else +// (e.g. the MacBook, /Users/lan), the mini profile's hardcoded +// /Users/lannguyensi/... source instead makes a pull step try to mkdir a +// path outside any real home the current user can write to — the EACCES +// this whole task starts from. Remapping removes both failure modes. +// +// rootDir/stateDir/remoteUrl get the same treatment for a related but +// distinct reason: nothing in the CLI reads them from the derived config +// today — resolveRunConfig's merge order (src/config/loader.ts) puts +// CLI-flag overrides last, so the --root-dir/--state-dir/--remote flags +// machineArgs below always passes win over whatever the config file says. +// Hermeticity for these three used to rest entirely on machineArgs always +// passing all three flags, with nothing asserting it — silently dropping +// one would have pushed straight into the REAL values the committed +// profile still carries verbatim in its config (rootDir: the operator's +// real memory directory, currently ~270 live files; remoteUrl: the real +// production bare repo; stateDir: the real state dir). Two things close +// that, for defense in depth: (1) remapAbsoluteSyncPathsIntoSandbox also +// rewrites rootDir/stateDir/remoteUrl in the derived config to the same +// sandboxed values passed as CLI flags (an observable no-op today given +// the merge order above, but it removes the live paths from the config +// file this test hands to --config entirely), and (2) machineArgs itself +// now asserts each of --root-dir/--state-dir/--remote resolves under the +// sandbox root before returning the CLI argv, so a future call site that +// drops or mistypes one of them fails this test immediately, before any +// CLI invocation, instead of silently reaching a real path. +// +// Between the CLI-flag-level guard in machineArgs and the config-level +// guard in assertSyncPathSourcesWithinSandbox, every path the CLI can act +// on in this test — --root-dir/--state-dir/--remote, the "." memory +// entry's source (relative to rootDir), and the machine-state/frictions +// entries' rewritten absolute sources — resolves under the sandbox root, +// so nothing this test does can touch a real machine's filesystem, +// regardless of what machine it runs on. The flags, the config rewrite, +// and the guard together sandbox rootDir/stateDir/remoteUrl; no single one +// of them is load-bearing alone. +// +// repositorySubdir and every other syncPaths field (kind, destination, +// ownerScoped) still come from the real files, unmodified — only the +// machine-state/frictions sources and rootDir/stateDir/remoteUrl are +// rewritten; the remap happens one layer up, in the config file this test +// hands to --config, with the CLI-flag values (machineArgs) as the +// actually-enforced defense. const test = require("node:test"); const assert = require("node:assert/strict"); const { readdirSync } = require("node:fs"); @@ -56,14 +114,47 @@ function listProfileFiles(): string[] { return files; } +// Resolves `candidate` and asserts it lands under `sandboxRoot` (exactly +// equal, or as a descendant path). The one choke point every +// sandbox-containment check in this file goes through, so +// assertSyncPathSourcesWithinSandbox (config-file syncPaths sources) and +// machineArgs (--root-dir/--state-dir/--remote CLI flags) apply the exact +// same resolve+startsWith logic instead of two implementations that could +// drift apart. +function assertPathWithinSandbox(candidate: string, sandboxRoot: string, label: string): void { + const resolvedSandboxRoot = path.resolve(sandboxRoot); + const resolvedCandidate = path.resolve(candidate); + const withinSandbox = + resolvedCandidate === resolvedSandboxRoot || resolvedCandidate.startsWith(`${resolvedSandboxRoot}${path.sep}`); + assert.ok( + withinSandbox, + `${label}: '${candidate}' resolves to '${resolvedCandidate}', OUTSIDE the test sandbox root ` + + `'${resolvedSandboxRoot}' — refusing to let this test touch a path that could be a real machine's.` + ); +} + +// Asserts rootDir/stateDir/remoteDir resolve under `sandboxRoot` BEFORE +// building the CLI argv, so a call site that drops or mistypes one of +// --root-dir/--state-dir/--remote (the flags that make the CLI ignore +// whatever the derived config's own rootDir/stateDir/remoteUrl say, per +// resolveRunConfig's merge order in src/config/loader.ts) fails this test +// immediately, before any CLI invocation, instead of silently reaching a +// real machine path — see the file-level comment above for why this +// mattered enough to add as a second, independent guard alongside the +// config-file rewrite in remapAbsoluteSyncPathsIntoSandbox. function machineArgs( profileName: string, configPath: string, rootDir: string, stateDir: string, remoteDir: string, + sandboxRoot: string, extra: string[] ): string[] { + assertPathWithinSandbox(rootDir, sandboxRoot, `machineArgs('${profileName}') --root-dir`); + assertPathWithinSandbox(stateDir, sandboxRoot, `machineArgs('${profileName}') --state-dir`); + assertPathWithinSandbox(remoteDir, sandboxRoot, `machineArgs('${profileName}') --remote`); + return [ "run", profileName, @@ -81,6 +172,135 @@ function machineArgs( ]; } +// The only two syncPaths destinations any committed profile gives a +// hardcoded machine-absolute 'source' (see profiles/mac-mini.json / +// macbook.json / linux.json's "/Users//.harness/{machine-state, +// frictions}" entries, pinned unmodified by the second test below). Every +// other entry (the "." memory entry) is already relative to --root-dir, which +// this test already sandboxes. +const HOME_ABSOLUTE_SYNC_DESTINATIONS = ["machine-state", "frictions"]; + +// Fails loudly (rather than silently reading/writing a real machine's files) +// if any syncPaths source in `settings` resolves outside `sandboxRoot` — an +// absolute source is checked directly; a relative source (e.g. the "." +// memory entry, or an escape attempt like "../../elsewhere") is resolved +// against `sandboxedRootDir` first, the same base directory the CLI itself +// would resolve it against once --root-dir is applied, so a relative source +// can't slip past this guard just by not being absolute. Called right after +// remapAbsoluteSyncPathsIntoSandbox below, before any CLI invocation, so a +// remap that silently no-ops (e.g. a future destination rename this test's +// remap logic doesn't know about) fails the test immediately instead of +// quietly falling through to a real path. +function assertSyncPathSourcesWithinSandbox( + settings: { syncPaths?: Array> }, + sandboxRoot: string, + sandboxedRootDir: string, + label: string +): void { + for (const entry of settings.syncPaths || []) { + const source = entry.source; + if (typeof source !== "string") { + continue; + } + const candidate = path.isAbsolute(source) ? source : path.resolve(sandboxedRootDir, source); + assertPathWithinSandbox( + candidate, + sandboxRoot, + `${label}: syncPaths entry with destination '${String(entry.destination)}' has source '${source}'` + ); + } +} + +// Loads the real committed profile at `realConfigPath`, rewrites the +// 'source' of its machine-state/frictions syncPaths entries (the only +// hardcoded machine-absolute paths any profile carries) to point inside a +// per-machine fake-$HOME under `sandboxRoot`, ALSO rewrites the profile's +// top-level rootDir/stateDir/remoteUrl to the sandboxed values the caller +// passes in (`sandboxedRootDir`/`sandboxedStateDir`/`sandboxedRemoteUrl` — +// the same values machineArgs will pass as --root-dir/--state-dir/--remote; +// see the file-level comment above for why this exists in addition to the +// CLI flags), asserts the result is fully sandboxed, and writes it to a new +// config file this test hands to --config in place of the real one. Every +// other field of every entry (kind, destination, ownerScoped, and the "." +// memory entry's source), plus repositorySubdir and everything else in the +// profile, is copied through unmodified. +function remapAbsoluteSyncPathsIntoSandbox( + realConfigPath: string, + sandboxRoot: string, + machineLabel: string, + sandboxedRootDir: string, + sandboxedStateDir: string, + sandboxedRemoteUrl: string +): { configPath: string; sandboxSourceByDestination: Record } { + const settings = JSON.parse(readText(realConfigPath)); + const fakeHome = path.join(sandboxRoot, `${machineLabel}-fake-home`); + const sandboxSourceByDestination: Record = {}; + + settings.syncPaths = (settings.syncPaths || []).map((entry: Record) => { + const destination = entry.destination as string; + if ( + !HOME_ABSOLUTE_SYNC_DESTINATIONS.includes(destination) || + typeof entry.source !== "string" || + !path.isAbsolute(entry.source as string) + ) { + return entry; + } + + const sandboxSource = path.join(fakeHome, ".harness", destination); + sandboxSourceByDestination[destination] = sandboxSource; + return { ...entry, source: sandboxSource }; + }); + + // Opaque-failure guard: a syncPaths entry the remap above never matched + // (e.g. a committed profile that dropped or renamed a machine-state/ + // frictions entry) would silently leave sandboxSourceByDestination missing + // that key, and every downstream test assertion keyed off it would either + // throw a confusing "undefined is not a directory" or — worse — read + // `undefined` and skip a check outright. Name the missing destination + // explicitly instead. + for (const destination of HOME_ABSOLUTE_SYNC_DESTINATIONS) { + assert.ok( + sandboxSourceByDestination[destination], + `remapped profile for '${machineLabel}' (source: ${realConfigPath}) has no syncPaths entry for ` + + `destination '${destination}' — remapAbsoluteSyncPathsIntoSandbox found nothing to rewrite, so this ` + + "destination's sandboxed source directory was never created." + ); + } + + // Rewrite rootDir/stateDir/remoteUrl into the config file itself. The CLI + // never reads these from the config here (the --root-dir/--state-dir/ + // --remote flags machineArgs passes always win — see the file-level + // comment), so this is an observable no-op today; it exists so the + // derived config this test hands to --config never carries the real + // operator paths verbatim, as defense in depth alongside the CLI-flag + // guard in machineArgs. + settings.rootDir = sandboxedRootDir; + settings.stateDir = sandboxedStateDir; + settings.remoteUrl = sandboxedRemoteUrl; + + assertSyncPathSourcesWithinSandbox( + settings, + sandboxRoot, + sandboxedRootDir, + `remapped profile for '${machineLabel}' (source: ${realConfigPath})` + ); + + const configPath = path.join(sandboxRoot, "configs", `${machineLabel}.json`); + writeText(configPath, `${JSON.stringify(settings, null, 2)}\n`); + return { configPath, sandboxSourceByDestination }; +} + +// Seeds a representative owner file (the ".json" convention +// collectLocalSyncFiles' ownerScoped filter looks for — see +// src/memory-sync/config.ts) inside a remapped sandbox source directory, so +// push actually has something to offer for that destination: an empty +// sandbox directory would make the machine-state/frictions sync paths a +// silent no-op, which is exactly the kind of silent skip this task's spec +// rules out. +function seedOwnerFile(sandboxSourceDir: string, ownerFileName: string, payload: Record): void { + writeText(path.join(sandboxSourceDir, ownerFileName), `${JSON.stringify(payload, null, 2)}\n`); +} + test("macbook and mac-mini profiles share one remote tree and see each other's pushes on pull (hermetic, local bare repo)", () => { const root = createSandbox("cross-machine-profiles"); const remoteDir = initBareRemote(root); @@ -93,16 +313,61 @@ test("macbook and mac-mini profiles share one remote tree and see each other's p const macbookState = path.join(root, "macbook-state"); const miniState = path.join(root, "mini-state"); - const macbookConfigPath = path.join(PROFILES_DIR, "macbook.json"); - const miniConfigPath = path.join(PROFILES_DIR, "mac-mini.json"); + // Remap the machine-absolute machine-state/frictions syncPaths sources + // (real committed values: mac-mini.json's + // "/Users/lannguyensi/.harness/{machine-state,frictions}", + // macbook.json's "/Users/lan/..." equivalents) into this sandbox before + // any CLI invocation. remapAbsoluteSyncPathsIntoSandbox asserts the result + // is fully sandboxed; nothing below this point can read or write a real + // machine's ~/.harness — see the file-level comment above for the leak + // this closes (agent-tasks 112a0864). + const macbookRemap = remapAbsoluteSyncPathsIntoSandbox( + path.join(PROFILES_DIR, "macbook.json"), + root, + "macbook", + macbookRoot, + macbookState, + remoteDir + ); + const miniRemap = remapAbsoluteSyncPathsIntoSandbox( + path.join(PROFILES_DIR, "mac-mini.json"), + root, + "mac-mini", + miniRoot, + miniState, + remoteDir + ); + const macbookConfigPath = macbookRemap.configPath; + const miniConfigPath = miniRemap.configPath; + + // Seed each machine's own owner file for the remapped machine-state/ + // frictions destinations so push has real, sandbox-owned content to offer + // — an empty remapped directory would make this coverage a silent no-op, + // which the task spec rules out. + seedOwnerFile(macbookRemap.sandboxSourceByDestination["machine-state"], "macbook.json", { + machine: "macbook", + note: "sandbox machine-state fixture (agent-tasks 112a0864)" + }); + seedOwnerFile(macbookRemap.sandboxSourceByDestination["frictions"], "macbook.json", { + machine: "macbook", + note: "sandbox frictions fixture (agent-tasks 112a0864)" + }); + seedOwnerFile(miniRemap.sandboxSourceByDestination["machine-state"], "mac-mini.json", { + machine: "mac-mini", + note: "sandbox machine-state fixture (agent-tasks 112a0864)" + }); + seedOwnerFile(miniRemap.sandboxSourceByDestination["frictions"], "mac-mini.json", { + machine: "mac-mini", + note: "sandbox frictions fixture (agent-tasks 112a0864)" + }); writeText(path.join(macbookRoot, "MEMORY.md"), "macbook memory\n"); writeText(path.join(miniRoot, "MEMORY.md"), "mini memory\n"); const macbookArgs = (...extra: string[]) => - machineArgs("macbook", macbookConfigPath, macbookRoot, macbookState, remoteDir, extra); + machineArgs("macbook", macbookConfigPath, macbookRoot, macbookState, remoteDir, root, extra); const miniArgs = (...extra: string[]) => - machineArgs("mac-mini", miniConfigPath, miniRoot, miniState, remoteDir, extra); + machineArgs("mac-mini", miniConfigPath, miniRoot, miniState, remoteDir, root, extra); // Seed: the mini (source of truth) pushes first, mirroring the real setup // order in docs/machine-setup.md. @@ -110,6 +375,55 @@ test("macbook and mac-mini profiles share one remote tree and see each other's p const miniSeedPayload = JSON.parse(miniSeed.stdout); assert.equal(miniSeedPayload.runs[0].status, "applied"); + // Cross-machine check, macbook <- mini direction: a fresh "macbook" CLI + // invocation (macbook's own remapped config, but a throwaway + // --root-dir/--state-dir it has never used before, so its 3-way-merge base + // starts empty for every destination) pulls immediately after the mini's + // seed push above. Its machine-state/frictions syncPaths sources are the + // SAME sandboxed absolute directories macbookRemap resolved (an absolute + // source is independent of --root-dir), so this exercises the real + // cross-machine materialization of the mini's owner file into macbook's + // sandboxed machine-state/frictions dirs — deliberately run here, before + // macbook's own real push below, because a machine's OWN push adopts + // whatever is already in the remote as its local base without ever writing + // it to disk (production behavior, not this task's concern): running the + // probe pull afterward would find a base===remote fast path and silently + // report nothing applied, which would make this check vacuous. + const macbookProbePull = runCli( + machineArgs( + "macbook", + macbookConfigPath, + path.join(root, "macbook-probe-workspace"), + path.join(root, "macbook-probe-state"), + remoteDir, + root, + ["--mode", "pull"] + ) + ); + const macbookProbePullPayload = JSON.parse(macbookProbePull.stdout); + assert.equal(macbookProbePullPayload.runs[0].status, "applied"); + assert.notEqual( + macbookProbePullPayload.runs[0].appliedFiles.length, + 0, + "macbook probe pull reported status 'applied' but appliedFiles is empty — status alone doesn't prove the " + + "mini's seed push actually materialized anything for macbook" + ); + + const macbookMachineStateDir = macbookRemap.sandboxSourceByDestination["machine-state"]; + const macbookFrictionsDir = macbookRemap.sandboxSourceByDestination["frictions"]; + assert.equal(fileExists(path.join(macbookMachineStateDir, "mac-mini.json")), true); + assert.equal( + JSON.parse(readText(path.join(macbookMachineStateDir, "mac-mini.json"))).machine, + "mac-mini", + "macbook's sandboxed machine-state dir did not receive the mini's owner file on pull" + ); + assert.equal(fileExists(path.join(macbookFrictionsDir, "mac-mini.json")), true); + assert.equal( + JSON.parse(readText(path.join(macbookFrictionsDir, "mac-mini.json"))).machine, + "mac-mini", + "macbook's sandboxed frictions dir did not receive the mini's owner file on pull" + ); + // A brand-new flat file appears on the MacBook — a plain edit, not a file // named in any hardcoded syncPaths list. writeText(path.join(macbookRoot, "e2e-sync-test.md"), "new file from macbook\n"); @@ -134,6 +448,27 @@ test("macbook and mac-mini profiles share one remote tree and see each other's p assert.equal(fileExists(path.join(miniRoot, "e2e-sync-test.md")), true); assert.equal(readText(path.join(miniRoot, "e2e-sync-test.md")), "new file from macbook\n"); + // The mini's pull above also carries the machine-state/frictions + // destinations (macbookPush offered macbook's own owner file for both, + // ownerScoped): the mini's SANDBOXED machine-state/frictions dirs must now + // hold macbook's owner file, proving the remapped syncPaths sources still + // exercise the real cross-machine sync path end to end, not just the + // memory destination. + const miniMachineStateDir = miniRemap.sandboxSourceByDestination["machine-state"]; + const miniFrictionsDir = miniRemap.sandboxSourceByDestination["frictions"]; + assert.equal(fileExists(path.join(miniMachineStateDir, "macbook.json")), true); + assert.equal( + JSON.parse(readText(path.join(miniMachineStateDir, "macbook.json"))).machine, + "macbook", + "mini's sandboxed machine-state dir did not receive macbook's owner file on pull" + ); + assert.equal(fileExists(path.join(miniFrictionsDir, "macbook.json")), true); + assert.equal( + JSON.parse(readText(path.join(miniFrictionsDir, "macbook.json"))).machine, + "macbook", + "mini's sandboxed frictions dir did not receive macbook's owner file on pull" + ); + // Reverse direction: the mini edits, the MacBook pulls. writeText(path.join(miniRoot, "mini-only-note.md"), "note from the mini\n"); const miniPush2 = runCli(miniArgs("--mode", "push"));