mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-12 06:06:37 +03:00
Address review: collision-resistant slugs, os.homedir() tilde expansion
- The per-project state dir key is now the readable separator-mapped slug plus an 8-hex sha256 of the resolved project path. The readable part alone is lossy (/x/my.app and /x/my-app both mapped to -x-my-app and shared hook state); the digest keeps distinct projects' cache and pending state apart while the dir name stays human-scannable. - Tilde roots now expand via os.homedir() instead of HOME/USERPROFILE with a '.' fallback. When no home dir can be determined, expansion is rejected and state falls back to the project-local default rather than anchoring under the hook process's working directory. - Tests updated to the digest-suffixed slug via a mirrored slugFor() helper, plus two new cases: colliding readable slugs get distinct state dirs, and the tilde form resolves identically to the explicit homedir-joined form. Prepared with AI assistance (Claude Code) under direction of 0xDarkMatter, per the maintainer-approved issue #422. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
committed by
Abdul Wahab
co-authored by
Claude Fable 5
parent
30b3628f5b
commit
cbd7870159
@@ -43,6 +43,7 @@
|
||||
* `cli/engine/detect-antipatterns.mjs` (running from source).
|
||||
*/
|
||||
|
||||
import crypto from 'node:crypto';
|
||||
import fs from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
@@ -220,20 +221,29 @@ export function getLocalConfigPath(cwd) {
|
||||
// disposable state relocates.
|
||||
// Read from process.env (not runHook's injected env): the cache root is a
|
||||
// machine-scoped setting like CURSOR_PROJECT_DIR, not a per-invocation
|
||||
// switch. Trim guards against stray whitespace in env files; `~/` expands to
|
||||
// the home dir (settings/env files hand it to Node unexpanded — same
|
||||
// treatment IMPECCABLE_HOOK_LOG gets in writeAuditLog); resolving both sides
|
||||
// makes the slug deterministic when callers hand in a trailing separator or
|
||||
// unnormalized cwd.
|
||||
// switch. Trim guards against stray whitespace in env files; `~/` (or the
|
||||
// Windows `~\` spelling) expands via os.homedir(), and when no home dir can
|
||||
// be determined the expansion is rejected — state falls back to the
|
||||
// project-local default rather than anchoring under the hook process's cwd.
|
||||
// Resolving both sides makes the slug deterministic when callers hand in a
|
||||
// trailing separator or unnormalized cwd. The slug is the readable
|
||||
// separator-mapped path PLUS an 8-hex sha256 of the resolved path: the
|
||||
// readable part alone is lossy (`/x/my.app` and `/x/my-app` would both map
|
||||
// to `-x-my-app` and share state), so the digest disambiguates while keeping
|
||||
// the dir name human-scannable.
|
||||
function hookStateDir(cwd) {
|
||||
const raw = process.env.IMPECCABLE_CACHE_ROOT;
|
||||
let root = typeof raw === 'string' ? raw.trim() : '';
|
||||
if (root.startsWith('~/') || root.startsWith('~\\') || root === '~') {
|
||||
root = path.join(process.env.HOME || process.env.USERPROFILE || '.', root.slice(2));
|
||||
let home = '';
|
||||
try { home = os.homedir() || ''; } catch { home = ''; }
|
||||
root = home ? path.join(home, root.slice(2)) : '';
|
||||
}
|
||||
if (root) {
|
||||
const slug = path.resolve(String(cwd)).replace(/[:\\/.]/g, '-');
|
||||
return path.join(path.resolve(root), slug);
|
||||
const resolved = path.resolve(String(cwd));
|
||||
const slug = resolved.replace(/[:\\/.]/g, '-');
|
||||
const digest = crypto.createHash('sha256').update(resolved).digest('hex').slice(0, 8);
|
||||
return path.join(path.resolve(root), `${slug}-${digest}`);
|
||||
}
|
||||
return path.join(cwd, '.impeccable');
|
||||
}
|
||||
|
||||
+37
-27
@@ -9,6 +9,7 @@
|
||||
|
||||
import { describe, it, beforeEach, afterEach } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import crypto from 'node:crypto';
|
||||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
import os from 'node:os';
|
||||
@@ -450,19 +451,40 @@ describe('IMPECCABLE_CACHE_ROOT relocates hook state (issue #422)', () => {
|
||||
assert.equal(getCachePath(cwd), path.join(cwd, '.impeccable', 'hook.cache.json'));
|
||||
});
|
||||
|
||||
// Mirrors hookStateDir's slug formula: readable separator-mapped path plus
|
||||
// an 8-hex sha256 disambiguator.
|
||||
function slugFor(p) {
|
||||
const resolved = path.resolve(p);
|
||||
const readable = resolved.replace(/[:\\/.]/g, '-');
|
||||
const digest = crypto.createHash('sha256').update(resolved).digest('hex').slice(0, 8);
|
||||
return `${readable}-${digest}`;
|
||||
}
|
||||
|
||||
it('relocates cache and pending under a per-project slug dir', () => {
|
||||
process.env.IMPECCABLE_CACHE_ROOT = cacheRoot;
|
||||
const slug = String(cwd).replace(/[:\\/.]/g, '-');
|
||||
assert.equal(getCachePath(cwd), path.join(cacheRoot, slug, 'hook.cache.json'));
|
||||
assert.equal(getPendingPath(cwd), path.join(cacheRoot, slug, 'hook.pending.json'));
|
||||
assert.equal(getCachePath(cwd), path.join(cacheRoot, slugFor(cwd), 'hook.cache.json'));
|
||||
assert.equal(getPendingPath(cwd), path.join(cacheRoot, slugFor(cwd), 'hook.pending.json'));
|
||||
});
|
||||
|
||||
it('slug maps separators, colons, and dots to hyphens', () => {
|
||||
it('slug maps separators, colons, and dots to hyphens, with a digest suffix', () => {
|
||||
process.env.IMPECCABLE_CACHE_ROOT = cacheRoot;
|
||||
const proj = path.join(cwd, 'my.app', 'v2');
|
||||
const slugDir = path.basename(path.dirname(getCachePath(proj)));
|
||||
assert.doesNotMatch(slugDir, /[:\\/.]/, 'no path-significant chars survive');
|
||||
assert.ok(slugDir.endsWith('my-app-v2'), `dots and separators map to hyphens (got ${slugDir})`);
|
||||
assert.match(slugDir, /my-app-v2-[0-9a-f]{8}$/, `readable slug + 8-hex digest (got ${slugDir})`);
|
||||
});
|
||||
|
||||
it('distinct projects whose readable slugs collide get distinct state dirs', () => {
|
||||
process.env.IMPECCABLE_CACHE_ROOT = cacheRoot;
|
||||
const dotted = path.join(cwd, 'my.app');
|
||||
const dashed = path.join(cwd, 'my-app');
|
||||
// Readable part is identical for both...
|
||||
assert.equal(
|
||||
path.resolve(dotted).replace(/[:\\/.]/g, '-'),
|
||||
path.resolve(dashed).replace(/[:\\/.]/g, '-'),
|
||||
);
|
||||
// ...but the digest keeps their hook state apart.
|
||||
assert.notEqual(path.dirname(getCachePath(dotted)), path.dirname(getCachePath(dashed)));
|
||||
});
|
||||
|
||||
it('trailing separators and relative segments slug to the same dir', () => {
|
||||
@@ -474,8 +496,7 @@ describe('IMPECCABLE_CACHE_ROOT relocates hook state (issue #422)', () => {
|
||||
|
||||
it('trims stray whitespace from the env value', () => {
|
||||
process.env.IMPECCABLE_CACHE_ROOT = ` ${cacheRoot} `;
|
||||
const slug = path.resolve(cwd).replace(/[:\\/.]/g, '-');
|
||||
assert.equal(getCachePath(cwd), path.join(cacheRoot, slug, 'hook.cache.json'));
|
||||
assert.equal(getCachePath(cwd), path.join(cacheRoot, slugFor(cwd), 'hook.cache.json'));
|
||||
});
|
||||
|
||||
it('persistCache degrades gracefully when the cache root is unusable', () => {
|
||||
@@ -495,24 +516,14 @@ describe('IMPECCABLE_CACHE_ROOT relocates hook state (issue #422)', () => {
|
||||
assert.equal(getLocalConfigPath(cwd), path.join(cwd, '.impeccable', 'config.local.json'));
|
||||
});
|
||||
|
||||
it('expands a leading ~/ against the home dir, like IMPECCABLE_HOOK_LOG', () => {
|
||||
const savedHome = process.env.HOME;
|
||||
const savedProfile = process.env.USERPROFILE;
|
||||
try {
|
||||
process.env.HOME = cacheRoot;
|
||||
delete process.env.USERPROFILE;
|
||||
process.env.IMPECCABLE_CACHE_ROOT = '~/impeccable-state';
|
||||
const slug = path.resolve(cwd).replace(/[:\\/.]/g, '-');
|
||||
assert.equal(
|
||||
getCachePath(cwd),
|
||||
path.join(cacheRoot, 'impeccable-state', slug, 'hook.cache.json'),
|
||||
);
|
||||
} finally {
|
||||
if (savedHome === undefined) delete process.env.HOME;
|
||||
else process.env.HOME = savedHome;
|
||||
if (savedProfile === undefined) delete process.env.USERPROFILE;
|
||||
else process.env.USERPROFILE = savedProfile;
|
||||
}
|
||||
it('expands a leading ~/ against os.homedir()', () => {
|
||||
// Property check without duplicating the expansion: the tilde form must
|
||||
// resolve identically to the explicit homedir-joined form.
|
||||
process.env.IMPECCABLE_CACHE_ROOT = path.join(os.homedir(), 'impeccable-state');
|
||||
const explicit = getCachePath(cwd);
|
||||
process.env.IMPECCABLE_CACHE_ROOT = '~/impeccable-state';
|
||||
assert.equal(getCachePath(cwd), explicit);
|
||||
assert.ok(explicit.startsWith(os.homedir()), 'anchored under the home dir');
|
||||
});
|
||||
|
||||
it('persistCache round-trips through the redirect dir and leaves the project root clean', () => {
|
||||
@@ -522,8 +533,7 @@ describe('IMPECCABLE_CACHE_ROOT relocates hook state (issue #422)', () => {
|
||||
assert.equal(persistCache(cwd, cache), true);
|
||||
|
||||
assert.equal(fs.existsSync(path.join(cwd, '.impeccable')), false, 'project root untouched');
|
||||
const slug = String(cwd).replace(/[:\\/.]/g, '-');
|
||||
assert.equal(fs.existsSync(path.join(cacheRoot, slug, 'hook.cache.json')), true);
|
||||
assert.equal(fs.existsSync(path.join(cacheRoot, slugFor(cwd), 'hook.cache.json')), true);
|
||||
|
||||
const reloaded = readCache(cwd);
|
||||
assert.equal(reloaded.sessions['sid-1'].files['/x/a.tsx'].editCount, 1);
|
||||
|
||||
Reference in New Issue
Block a user