root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(review): resolve PR feedback on resolveGlobalDir guard

Browse Source 
- Avoid the legacy-migration side effect on the hot path: run the cheap,
  read-only checks (basename === ".fusion" && parent has .git) FIRST, and
  only call resolveGlobalDirForHome() (which can perform a one-time rename)
  when a dir actually looks project-local.
- Normalize paths before the home-dir comparison (realpathSync when present,
  else resolve) so a trailing slash, doubled separator, or symlinked home
  doesn't make the legitimate home global dir trip the guard.
- Harden the browse-directory regression test: the mock returns a global dir
  DISTINCT from getFusionDir() and asserts CentralCore is constructed with the
  global dir, so a revert to getFusionDir() now fails the test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-06-25 23:17:02 -07:00
parent 84ec10d1a7
commit 9fd4dedcae
2 changed files with 31 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

40
packages/core/src/global-settings.ts
View File

@@ -14,9 +14,9 @@
*/
import { homedir } from "node:os";
import { basename, dirname, join } from "node:path";
import { basename, dirname, join, resolve } from "node:path";
import { mkdir, readFile, writeFile, rename, chmod } from "node:fs/promises";
import { existsSync, mkdirSync, renameSync } from "node:fs";
import { existsSync, mkdirSync, realpathSync, renameSync } from "node:fs";
import type { GlobalSettings } from "./types.js";
import { DEFAULT_GLOBAL_SETTINGS } from "./types.js";
import { sanitizeCliAgentsSettings } from "./settings-schema.js";
@@ -98,20 +98,32 @@ export function resolveGlobalDir(dir?: string): string {
FNXC:GlobalDirGuard 2026-06-25-22:55:
The heuristic is intentionally conservative but can't perfectly distinguish a project `.fusion` from a legitimately version-controlled custom global dir (e.g. a dotfiles repo with `~/dotfiles/.fusion` + `.git`). To avoid hard-crashing that rare setup, honor an explicit opt-out env var `FUSION_ALLOW_PROJECT_LOCAL_GLOBAL_DIR=true`. This is not reachable via normal production call sites (they resolve to ~/.fusion); it only matters for operators who deliberately configure a custom global dir inside a repo.
FNXC:GlobalDirGuard 2026-06-26-06:25:
Order matters and the home-dir comparison must be normalized:
- Run the CHEAP, read-only checks first (basename is `.fusion` AND its parent contains a `.git`). Only if both hold do we call `resolveGlobalDirForHome()` — which can perform a one-time legacy-dir rename — so we never trigger that filesystem side effect on the hot path (every explicit-dir call, e.g. getGlobalSettingsDir() in dashboard routes, previously hit it).
- Compare against the home global dir using normalized real paths (realpathSync when the path exists, else resolve()), so a trailing slash, doubled separator, or symlinked home dir doesn't make the legitimate home global dir look like a foreign project dir and trip the guard.
*/
if (process.env.VITEST !== "true" && process.env.FUSION_ALLOW_PROJECT_LOCAL_GLOBAL_DIR !== "true") {
const homeGlobalDir = resolveGlobalDirForHome(getHomeDir());
const looksLikeProjectFusionDir =
dir !== homeGlobalDir &&
basename(dir) === ".fusion" &&
existsSync(join(dirname(dir), ".git"));
if (looksLikeProjectFusionDir) {
throw new Error(
`resolveGlobalDir(): refusing project-local '.fusion' directory '${dir}' for the central/global store. ` +
"This would create a stray per-project central database seeded with default global settings and silently reset them. " +
"Pass the resolved global dir (or omit the argument so it defaults to ~/.fusion); see TaskStore.getGlobalSettingsDir(). " +
"If this really is your intended global dir (e.g. a version-controlled dotfiles repo), set FUSION_ALLOW_PROJECT_LOCAL_GLOBAL_DIR=true to override.",
);
const isFusionDirInsideRepo =
basename(dir) === ".fusion" && existsSync(join(dirname(dir), ".git"));
if (isFusionDirInsideRepo) {
const homeGlobalDir = resolveGlobalDirForHome(getHomeDir());
const normalize = (p: string): string => {
try {
return realpathSync.native(p);
} catch {
return resolve(p);
}
};
if (normalize(dir) !== normalize(homeGlobalDir)) {
throw new Error(
`resolveGlobalDir(): refusing project-local '.fusion' directory '${dir}' for the central/global store. ` +
"This would create a stray per-project central database seeded with default global settings and silently reset them. " +
"Pass the resolved global dir (or omit the argument so it defaults to ~/.fusion); see TaskStore.getGlobalSettingsDir(). " +
"If this really is your intended global dir (e.g. a version-controlled dotfiles repo), set FUSION_ALLOW_PROJECT_LOCAL_GLOBAL_DIR=true to override.",
);
}
}
}
return dir;

7
packages/dashboard/src/__tests__/browse-directory-routes.test.ts
View File

@@ -40,6 +40,7 @@ vi.mock("@fusion/core", async () => {
// Import after mocking
import { browseDirectory } from "../../app/api.js";
import { CentralCore } from "@fusion/core";
function mockFetchResponse(
ok: boolean,
@@ -71,9 +72,9 @@ class MockStoreForRoutes extends EventEmitter {
return "/tmp/fn-944/.fusion";
}
// FNXC:GlobalDirGuard 2026-06-25-23:10: Routes resolve the global central dir via getGlobalSettingsDir(); mock mirrors getFusionDir() (CentralCore is mocked) so route behavior matches pre-change.
// FNXC:GlobalDirGuard 2026-06-26-06:25: Return a global dir DISTINCT from getFusionDir() so the regression test can assert the route constructs CentralCore with the global dir — a future revert to getFusionDir() then fails the CentralCore-constructor assertion below instead of silently passing.
getGlobalSettingsDir(): string {
return this.getFusionDir();
return "/tmp/fn-944/.fusion-global";
}
getDatabase() {
@@ -154,6 +155,8 @@ describe("GET /api/browse-directory route handler", () => {
expect(mockListNodes).toHaveBeenCalled();
expect(mockClose).toHaveBeenCalled();
expect(globalThis.fetch).not.toHaveBeenCalled();
// FNXC:GlobalDirGuard 2026-06-26-06:25: The node-aware route must build CentralCore from the GLOBAL dir, never the project `.fusion/`. Asserting the distinct global path makes a revert to getFusionDir() ("/tmp/fn-944/.fusion") fail here.
expect(vi.mocked(CentralCore)).toHaveBeenCalledWith("/tmp/fn-944/.fusion-global");
});
});