Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 37 additions & 2 deletions src/lib/workspace.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { afterEach, beforeEach, describe, expect, test } from 'bun:test';
import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
import { existsSync, mkdirSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { findWorkspace, getWorkspaceConfig, scanAgents } from './workspace.js';
import { findWorkspace, genieHome, getWorkspaceConfig, scanAgents } from './workspace.js';

let testDir: string;
let fakeGenieHome: string;
Expand Down Expand Up @@ -122,6 +122,41 @@ describe('workspaceRoot persistence', () => {
// (if the file doesn't exist at all, that's also a pass — nothing was persisted)
});

test('canonicalizes symlinked tmp paths so the guard cannot be bypassed', () => {
// Regression guard for the macOS /tmp → /private/tmp case. path.resolve()
// does NOT follow symlinks, so a naive prefix check against a non-canonical
// path would miss. realpathSync() canonicalizes both sides.
const real = join(testDir, 'real-workspace');
const link = join(testDir, 'linked-workspace');
mkdirSync(real, { recursive: true });
makeWorkspace(real);
try {
symlinkSync(real, link, 'dir');
} catch {
// Some CI environments disallow symlink creation; skip gracefully.
return;
}

// findWorkspace called via the symlink path — without realpathSync the
// tmp-guard would fail to match, with realpathSync it matches correctly.
const result = findWorkspace(link);
expect(result).not.toBeNull();

// Either the config file doesn't exist at all, or it exists with no workspaceRoot.
// Both mean "nothing was persisted" — which is what we want.
const configPath = join(fakeGenieHome, 'config.json');
if (existsSync(configPath)) {
const config = JSON.parse(readFileSync(configPath, 'utf-8'));
expect(config.workspaceRoot).toBeUndefined();
}
});

test('genieHome() reflects GENIE_HOME env override', () => {
// Sanity check: the exported helper resolves dynamically so callers in other
// modules (e.g. serve.ts warning messages) see the same value as workspace.ts.
expect(genieHome()).toBe(fakeGenieHome);
});

test('clears stale workspaceRoot from config when the saved path is gone', () => {
// Simulate a prior run that persisted a path which has since vanished.
const vanished = join(testDir, 'was-here-now-gone');
Expand Down
26 changes: 20 additions & 6 deletions src/lib/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* If the cwd passes through `agents/<name>/`, the agent name is extracted.
*/

import { existsSync, mkdirSync, readFileSync, readdirSync, writeFileSync } from 'node:fs';
import { existsSync, mkdirSync, readFileSync, readdirSync, realpathSync, writeFileSync } from 'node:fs';
import { homedir, tmpdir } from 'node:os';
import { dirname, join, resolve, sep } from 'node:path';

Expand Down Expand Up @@ -63,15 +63,29 @@ export function findWorkspace(cwd?: string): WorkspaceInfo | null {
}

/** Resolved lazily so GENIE_HOME env overrides in tests take effect. */
function genieHome(): string {
export function genieHome(): string {
return process.env.GENIE_HOME ?? join(homedir(), '.genie');
}

/** True if `root` is under the OS temp directory (avoids persisting test workspaces). */
/**
* True if `root` is under the OS temp directory (avoids persisting test workspaces).
*
* Uses `realpathSync` to canonicalize both paths before comparing — on macOS
* `/tmp` is a symlink to `/private/tmp`, and `tmpdir()` returns `/var/folders/…`,
* so a plain string prefix check against non-canonical paths would bypass the guard.
*
* Fails CLOSED: if canonicalization fails (path doesn't exist, permission error),
* treat as temp and refuse to persist — the saveWorkspaceRoot caller's goal is
* to be conservative about what lands in the global config.
*/
function isTempPath(root: string): boolean {
const normalizedTmp = resolve(tmpdir());
const normalizedRoot = resolve(root);
return normalizedRoot === normalizedTmp || normalizedRoot.startsWith(normalizedTmp + sep);
try {
const canonicalTmp = realpathSync(tmpdir());
const canonicalRoot = realpathSync(root);
return canonicalRoot === canonicalTmp || canonicalRoot.startsWith(canonicalTmp + sep);
} catch {
return true;
}
}

function saveWorkspaceRoot(root: string): void {
Expand Down
6 changes: 4 additions & 2 deletions src/term-commands/serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -369,12 +369,14 @@ const handles: DaemonHandles = { schedulerHandle: null, agentWatcher: null };
/** Sync agent directory from workspace and start file watcher. */
async function startAgentSync(): Promise<{ close: () => void } | null> {
try {
const { findWorkspace } = require('../lib/workspace.js') as typeof import('../lib/workspace.js');
const { findWorkspace, genieHome } = require('../lib/workspace.js') as typeof import('../lib/workspace.js');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Importing genieHome here shadows the local genieHome function defined at line 31. Since both functions have the same implementation, this import is redundant. To improve maintainability, you should consider removing the local definition at line 31 and using the exported one from workspace.ts throughout the file, but for now, removing it from this import will resolve the shadowing. This dynamic import pattern is consistent with the repository's approach to breaking circular dependencies.

Suggested change
const { findWorkspace, genieHome } = require('../lib/workspace.js') as typeof import('../lib/workspace.js');
const { findWorkspace } = require('../lib/workspace.js') as typeof import('../lib/workspace.js');
References
  1. Use dynamic imports (require() or import()) to break circular dependency cycles that would otherwise occur at module load time.

const ws = findWorkspace();
if (!ws) {
// Loud failure — silent return used to hide the whole discovery subsystem
// when serve booted from outside a workspace (or with a stale saved root).
console.warn(' Agent sync: DISABLED — no workspace found from cwd or ~/.genie/config.json');
const { join } = require('node:path') as typeof import('node:path');
const configPath = join(genieHome(), 'config.json');
Comment on lines +377 to +378

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The join function is already imported from node:path at the top of the file (line 23). This dynamic require is redundant and can be removed since the module is already loaded at the top level.

Suggested change
const { join } = require('node:path') as typeof import('node:path');
const configPath = join(genieHome(), 'config.json');
const configPath = join(genieHome(), 'config.json');

console.warn(` Agent sync: DISABLED — no workspace found from cwd or ${configPath}`);
console.warn(' Fix: `cd <workspace> && genie serve restart`, or run `genie init` to bootstrap one');
return null;
}
Expand Down
Loading