diff --git a/README.md b/README.md index 4498905..34c0e05 100644 --- a/README.md +++ b/README.md @@ -243,6 +243,7 @@ diffity list --json # machine-readable output | -------------- | ------------------------------------------------------------------------- | | `DIFFITY_HOST` | Hostname used in the printed URL (default: `localhost`). | | `DIFFITY_BIND` | Interface the server listens on (default: `127.0.0.1`). | +| `DIFFITY_DATA_DIR` | Where review notes are kept (default: `~/.diffity/`). | Useful when running diffity inside a VM or container and opening it from another machine: @@ -254,6 +255,27 @@ The server has no authentication: anything that can reach it can read the diff, repository's files and the review comments. Only widen `DIFFITY_BIND` on a network you trust. +## Where review notes live + +Review threads, walkthroughs and sessions are kept in a SQLite database. By default that is +`~/.diffity//reviews.db`, one per repository. + +A project can keep its own instead, which is what you want when several worktrees of the same +repository each need their own notes, or when the notes should travel with the project rather +than the machine. Commit a `.diffity.json` at the repository root: + +```json +{ "dataDir": "../.diffity" } +``` + +Relative paths resolve against the repository root, absolute paths are used as given, and +`DIFFITY_DATA_DIR` overrides both. A directory chosen this way is used as-is — no hashed +subdirectory, since there is nothing to disambiguate. + +Point it **outside** the working tree, or add it to `.gitignore`. Otherwise the notes show up as +untracked files in the very diff you are reviewing; diffity warns on startup when that happens. +The database quotes the code under review, so it is created readable only by you. + ## License [PolyForm Shield 1.0.0](./LICENSE) © [Kamran Ahmed](https://x.com/kamrify) diff --git a/packages/cli/src/db.ts b/packages/cli/src/db.ts index a7113a5..adcad7e 100644 --- a/packages/cli/src/db.ts +++ b/packages/cli/src/db.ts @@ -1,3 +1,4 @@ +import { chmodSync } from 'node:fs'; import { createRequire } from 'node:module'; import { join } from 'node:path'; import type { DatabaseSync, SQLInputValue } from 'node:sqlite'; @@ -19,6 +20,18 @@ function loadSqlite(): { DatabaseSync: new (path: string) => DatabaseSync } { } } +// The database holds `anchor_content` — the actual source lines a comment is attached to — so +// it must not be world-readable. WAL and shared-memory siblings hold the same content. +function restrictToOwner(dbPath: string): void { + for (const path of [dbPath, `${dbPath}-wal`, `${dbPath}-shm`]) { + try { + chmodSync(path, 0o600); + } catch { + // Not all siblings exist at every moment; the ones that do are what matter. + } + } +} + export function getDb(): DatabaseSync { if (db) { return db; @@ -27,6 +40,7 @@ export function getDb(): DatabaseSync { const { DatabaseSync: Database } = loadSqlite(); const dbPath = join(getDiffityDir(), 'reviews.db'); db = new Database(dbPath); + restrictToOwner(dbPath); db.exec('PRAGMA journal_mode = WAL'); db.exec('PRAGMA foreign_keys = ON'); migrateDb(db); diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index f3abab4..62f81f7 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -3,7 +3,7 @@ import { createHash } from 'node:crypto'; import { createRequire } from 'node:module'; import open from 'open'; import pc from 'picocolors'; -import { isGitRepo, isValidGitRef, getRepoRoot, getRepoName, normalizeRef, WORKING_TREE_REFS } from '@diffity/git'; +import { isGitRepo, isValidGitRef, getRepoRoot, getRepoName, normalizeRef, getDiffityDirPath, isDataDirUntracked, WORKING_TREE_REFS } from '@diffity/git'; import type { PrBase } from '@diffity/github'; import { isGitHubPrUrl, @@ -235,6 +235,12 @@ range syntax (main..feature, main...feature) also work.`) } const repoRoot = getRepoRoot(); + + if (!opts.quiet && isDataDirUntracked()) { + console.log(pc.yellow(` Note: review notes are kept in ${getDiffityDirPath()}, which git does not ignore.`)); + console.log(pc.dim(' Add it to .gitignore so they stay out of the diff you are reviewing.')); + } + const repoHash = createHash('sha256').update(repoRoot).digest('hex').slice(0, 12); const repoName = getRepoName(); diff --git a/packages/git/src/config.ts b/packages/git/src/config.ts new file mode 100644 index 0000000..9c1ecb5 --- /dev/null +++ b/packages/git/src/config.ts @@ -0,0 +1,49 @@ +import { readFileSync, existsSync } from 'node:fs'; +import { createHash } from 'node:crypto'; +import { isAbsolute, join, resolve } from 'node:path'; + +export const REPO_CONFIG_FILE = '.diffity.json'; + +export interface RepoConfig { + /** + * Where review threads, walkthroughs and sessions are kept. Relative paths resolve against + * the repository root, so a project can keep its review notes with itself. + */ + dataDir?: string; +} + +export function readRepoConfig(repoRoot: string): RepoConfig { + const path = join(repoRoot, REPO_CONFIG_FILE); + if (!existsSync(path)) { + return {}; + } + + try { + const parsed = JSON.parse(readFileSync(path, 'utf-8')) as RepoConfig; + return typeof parsed?.dataDir === 'string' ? { dataDir: parsed.dataDir } : {}; + } catch { + // A malformed config must not stop a review; the default is always usable. + return {}; + } +} + +/** + * The hashed subdirectory only exists to keep repositories apart inside the shared default + * location. A data directory chosen for one project needs no such disambiguation, so it is + * used as given — `.diffity/reviews.db` rather than `.diffity//reviews.db`. + */ +export function resolveDataDir(input: { + repoRoot: string; + homeDir: string; + envDir?: string; + configDir?: string; +}): string { + const chosen = input.envDir?.trim() || input.configDir?.trim(); + + if (chosen) { + return isAbsolute(chosen) ? chosen : resolve(input.repoRoot, chosen); + } + + const hash = createHash('sha256').update(input.repoRoot).digest('hex').slice(0, 12); + return join(input.homeDir, '.diffity', hash); +} diff --git a/packages/git/src/index.ts b/packages/git/src/index.ts index 2031e21..a876586 100644 --- a/packages/git/src/index.ts +++ b/packages/git/src/index.ts @@ -1,9 +1,11 @@ export type { Commit, RepoInfo } from './types.js'; export type { RefCapabilities } from './repo.js'; -export { isGitRepo, getRepoRoot, getRepoName, getCurrentBranch, getRepoInfo, getHeadHash, getDiffityDir, getDiffityDirPath, getRefCapabilities, isValidGitRef } from './repo.js'; +export { isGitRepo, getRepoRoot, getRepoName, getCurrentBranch, getRepoInfo, getHeadHash, getDiffityDir, getDiffityDirPath, isDataDirUntracked, getRefCapabilities, isValidGitRef } from './repo.js'; export { getDiff, getDiffFiles, getDiffStat, getDiffStatForRef, getUntrackedFiles, getUntrackedDiff, getFileContent, getFileLineCount, getMergeBase, normalizeRef, resolveBaseRef, resolveThroughUpstream, resolveDiffArgs, resolveRef, revertFile, revertHunk, WORKING_TREE_REFS } from './diff.js'; export type { RefDiffArgs } from './diff.js'; export { getStagedFiles, getUnstagedFiles, isDirty } from './status.js'; export { getRecentCommits } from './commits.js'; +export { readRepoConfig, resolveDataDir, REPO_CONFIG_FILE } from './config.js'; +export type { RepoConfig } from './config.js'; export { getTree, getTreeEntries, getTreeFingerprint, getWorkingTreeFileContent, getWorkingTreeRawFile, resolveInRepo } from './tree.js'; export type { TreeEntry } from './tree.js'; diff --git a/packages/git/src/repo.ts b/packages/git/src/repo.ts index aded22e..eabeffa 100644 --- a/packages/git/src/repo.ts +++ b/packages/git/src/repo.ts @@ -1,9 +1,9 @@ import { execFileSync, execSync } from 'node:child_process'; -import { createHash } from 'node:crypto'; import { mkdirSync } from 'node:fs'; -import { join } from 'node:path'; +import { sep } from 'node:path'; import { homedir } from 'node:os'; import { exec } from './exec.js'; +import { readRepoConfig, resolveDataDir } from './config.js'; import { WORKING_TREE_REFS } from './diff.js'; import type { RepoInfo } from './types.js'; @@ -47,16 +47,41 @@ export function getHeadHash(): string { export function getDiffityDirPath(): string { const repoRoot = getRepoRoot(); - const hash = createHash('sha256').update(repoRoot).digest('hex').slice(0, 12); - return join(homedir(), '.diffity', hash); + return resolveDataDir({ + repoRoot, + homeDir: homedir(), + envDir: process.env.DIFFITY_DATA_DIR, + configDir: readRepoConfig(repoRoot).dataDir, + }); } export function getDiffityDir(): string { const dir = getDiffityDirPath(); - mkdirSync(dir, { recursive: true }); + // Review notes quote the code under review, so they are not readable by other accounts. + mkdirSync(dir, { recursive: true, mode: 0o700 }); return dir; } +/** + * True when the data directory sits inside the working tree without git ignoring it, which + * would otherwise show review notes as untracked changes in the very diff being reviewed. + */ +export function isDataDirUntracked(): boolean { + const dir = getDiffityDirPath(); + const repoRoot = getRepoRoot(); + + if (!dir.startsWith(repoRoot + sep)) { + return false; + } + + try { + execFileSync('git', ['check-ignore', '--quiet', dir], { stdio: 'pipe' }); + return false; + } catch { + return true; + } +} + export function isValidGitRef(ref: string): boolean { if (ref.includes('...')) { const parts = ref.split('...'); diff --git a/packages/git/tests/data-dir.test.ts b/packages/git/tests/data-dir.test.ts new file mode 100644 index 0000000..efe18d6 --- /dev/null +++ b/packages/git/tests/data-dir.test.ts @@ -0,0 +1,84 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { readRepoConfig, resolveDataDir } from '../src/config.js'; + +let repoRoot: string; +const homeDir = '/home/someone'; + +beforeEach(() => { + repoRoot = mkdtempSync(join(tmpdir(), 'diffity-datadir-')); +}); + +afterEach(() => { + rmSync(repoRoot, { recursive: true, force: true }); +}); + +describe('resolveDataDir', () => { + it('keeps repositories apart under the shared default', () => { + const a = resolveDataDir({ repoRoot: '/repos/a', homeDir }); + const b = resolveDataDir({ repoRoot: '/repos/b', homeDir }); + + expect(a).toMatch(/^\/home\/someone\/\.diffity\/[0-9a-f]{12}$/); + expect(a).not.toBe(b); + }); + + it('is stable for the same repository', () => { + expect(resolveDataDir({ repoRoot: '/repos/a', homeDir })).toBe( + resolveDataDir({ repoRoot: '/repos/a', homeDir }), + ); + }); + + it('uses a configured relative directory inside the project, with no hash', () => { + expect(resolveDataDir({ repoRoot: '/repos/a', homeDir, configDir: '.diffity' })).toBe( + '/repos/a/.diffity', + ); + }); + + it('uses a configured absolute directory as given', () => { + expect(resolveDataDir({ repoRoot: '/repos/a', homeDir, configDir: '/srv/notes' })).toBe( + '/srv/notes', + ); + }); + + it('lets the environment win over the repository config', () => { + expect( + resolveDataDir({ repoRoot: '/repos/a', homeDir, envDir: '/srv/env', configDir: '.diffity' }), + ).toBe('/srv/env'); + }); + + it('ignores blank values rather than resolving to the repository root', () => { + expect(resolveDataDir({ repoRoot: '/repos/a', homeDir, envDir: ' ' })).toMatch(/\.diffity\//); + }); +}); + +describe('readRepoConfig', () => { + it('reads dataDir', () => { + writeFileSync(join(repoRoot, '.diffity.json'), JSON.stringify({ dataDir: '.notes' })); + + expect(readRepoConfig(repoRoot)).toEqual({ dataDir: '.notes' }); + }); + + it('is empty when there is no config', () => { + expect(readRepoConfig(repoRoot)).toEqual({}); + }); + + it('survives malformed json rather than failing the review', () => { + writeFileSync(join(repoRoot, '.diffity.json'), '{ not json'); + + expect(readRepoConfig(repoRoot)).toEqual({}); + }); + + it('ignores a dataDir of the wrong type', () => { + writeFileSync(join(repoRoot, '.diffity.json'), JSON.stringify({ dataDir: 42 })); + + expect(readRepoConfig(repoRoot)).toEqual({}); + }); + + it('ignores a directory instead of a config file', () => { + mkdirSync(join(repoRoot, '.diffity.json')); + + expect(readRepoConfig(repoRoot)).toEqual({}); + }); +});