Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
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
155 changes: 155 additions & 0 deletions packages/core/src/config/storage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -691,3 +691,158 @@ describe('Storage – runtime base dir async context isolation', () => {
});
});
});

describe('Storage – getAuditFallbackDir', () => {
const originalEnv = process.env['QWEN_HOME'];
let home: string;

beforeEach(() => {
home = actualFs.mkdtempSync(path.join(os.tmpdir(), 'qwen-home-test-'));
process.env['QWEN_HOME'] = home;
// The file-wide mock replaces realpathSync with a bare vi.fn(), which
// answers `undefined`. Give it the real function's contract instead:
// production code must not carry a branch that exists only to tolerate a
// test double.
mockRealpathSync.mockImplementation((p: unknown) =>
actualFs.realpathSync(String(p)),
);
});

afterEach(() => {
actualFs.rmSync(home, { recursive: true, force: true });
if (originalEnv === undefined) {
delete process.env['QWEN_HOME'];
} else {
process.env['QWEN_HOME'] = originalEnv;
}
});

it('lands under QWEN_HOME/audits/<project hash>', () => {
const dir = Storage.getAuditFallbackDir('/some/project');
expect(path.dirname(path.dirname(dir))).toBe(home);
expect(path.basename(path.dirname(dir))).toBe('audits');
expect(path.basename(dir)).toMatch(/^[0-9a-f]{64}$/);
expect(actualFs.statSync(dir).isDirectory()).toBe(true);
});

it('creates the landing 0700 so quoted module content stays private', () => {
const mode = actualFs.statSync(Storage.getAuditFallbackDir('/p')).mode;
// On Windows mkdirSync's mode is a no-op and libuv emulates permission
// bits by duplicating owner bits to group/other.
if (process.platform !== 'win32') {
expect(mode & 0o077).toBe(0);
expect(mode & 0o700).toBe(0o700);
}
});

it('separates projects and is idempotent', () => {
const first = Storage.getAuditFallbackDir('/project/a');
const second = Storage.getAuditFallbackDir('/project/b');
expect(first).not.toBe(second);
expect(Storage.getAuditFallbackDir('/project/a')).toBe(first);
});

it('refuses a landing planted as a symlink instead of adopting it', () => {
if (process.platform === 'win32') return;
const decoy = actualFs.mkdtempSync(path.join(os.tmpdir(), 'audit-decoy-'));
const audits = path.join(home, 'audits');
actualFs.mkdirSync(audits, { recursive: true });
// Predict the leaf the way the audited agent can: the hash is a pure
// function of the project root.
const leaf = Storage.getAuditFallbackDir('/predictable');
actualFs.rmSync(leaf, { recursive: true, force: true });
actualFs.symlinkSync(decoy, leaf);
try {
expect(() => Storage.getAuditFallbackDir('/predictable')).toThrow(
/not a directory/,
);
} finally {
actualFs.rmSync(leaf, { force: true });
actualFs.rmSync(decoy, { recursive: true, force: true });
}
});

it('refuses an audits PARENT planted as a symlink, which relocates the whole landing', () => {
// mkdirSync(recursive) follows symlinks in every component ABOVE the
// leaf, and lstat refuses to follow only the FINAL one — so a leaf-only
// check cannot see a redirected parent. Planting `audits` is one `ln -s`
// with no race: ~/.qwen exists long before `audits` does.
if (process.platform === 'win32') return;
const attacker = actualFs.mkdtempSync(
path.join(os.tmpdir(), 'audit-attacker-'),
);
actualFs.symlinkSync(attacker, path.join(home, 'audits'));
try {
expect(() => Storage.getAuditFallbackDir('/any/project')).toThrow(
/audit artifact directory .* is not a directory/,
);
// Nothing was created inside the planter's directory.
expect(actualFs.readdirSync(attacker)).toEqual([]);
} finally {
actualFs.rmSync(path.join(home, 'audits'), { force: true });
actualFs.rmSync(attacker, { recursive: true, force: true });
}
});

it('refuses a landing holding a symlink child, which redirects writes out of it', () => {
// Validating the leaf alone leaves the escape open: artifacts land BELOW
// it, and mkdirSync treats a symlink-to-directory as the directory, so
// everything written "inside" goes wherever the link points while the
// leaf keeps passing every check.
if (process.platform === 'win32') return;
const escape = actualFs.mkdtempSync(path.join(os.tmpdir(), 'audit-out-'));
const leaf = Storage.getAuditFallbackDir('/with-child');
actualFs.symlinkSync(escape, path.join(leaf, 'audit-2026-01-01.sidecar'));
try {
expect(() => Storage.getAuditFallbackDir('/with-child')).toThrow(
/contains a symlink/,
);
} finally {
actualFs.rmSync(escape, { recursive: true, force: true });
}
});

it('refuses a landing holding a hardlinked file', () => {
if (process.platform === 'win32') return;
const leaf = Storage.getAuditFallbackDir('/with-hardlink');
const twin = path.join(home, 'twin.md');
actualFs.writeFileSync(twin, 'planted\n');
actualFs.linkSync(twin, path.join(leaf, '2026-01-01-000000-mod.md'));
expect(() => Storage.getAuditFallbackDir('/with-hardlink')).toThrow(
/hardlinked file/,
);
});

it('keeps adopting a landing that holds a previous run own artifacts', () => {
// The landing is REUSED: the report and its sidecar are the durable
// artifacts, so refusing a non-empty landing would refuse every run
// after the first.
const leaf = Storage.getAuditFallbackDir('/reused');
actualFs.writeFileSync(
path.join(leaf, '2026-01-01-000000-mod.md'),
'# r\n',
);
actualFs.mkdirSync(path.join(leaf, 'audit-2026-01-01.sidecar'), {
recursive: true,
});
expect(Storage.getAuditFallbackDir('/reused')).toBe(leaf);
});

it('is stable across symlink spellings of the same directory', () => {
// macOS `/var` → `/private/var`: plan-files and guard-check must hash the
// same logical directory to the same fallback root whichever spelling
// arrives, or the relocation-containment check spuriously fails.
if (process.platform === 'win32') return;
const real = actualFs.mkdtempSync(path.join(os.tmpdir(), 'audit-real-'));
const link = path.join(os.tmpdir(), `audit-link-${Date.now()}`);
try {
actualFs.symlinkSync(real, link);
expect(Storage.getAuditFallbackDir(link)).toBe(
Storage.getAuditFallbackDir(actualFs.realpathSync(real)),
);
} finally {
actualFs.rmSync(link, { force: true });
actualFs.rmSync(real, { recursive: true, force: true });
}
});
});
147 changes: 147 additions & 0 deletions packages/core/src/config/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,153 @@ export class Storage {
return path.join(Storage.getGlobalQwenDir(), ARENA_DIR_NAME);
}

/**
* Outside-repo landing for /audit reports and sidecars when the audited
* repository's ignore state cannot keep them out of version control.
* Per-user and per-project, honoring the QWEN_HOME override; 0700 so the
* quoted (possibly exploitable) module content stays private to the user.
*/
static getAuditFallbackDir(projectRoot: string): string {
// Resolve symlinks before hashing so the fallback root is stable across
// spellings of the same directory (macOS `/var` → `/private/var`):
// plan-files, guard-check, and the SKILL relocation must all agree on
// one root, or the relocation-containment check spuriously fails.
let resolved = projectRoot;
try {
resolved = fs.realpathSync(projectRoot);
} catch {
// Unresolvable (e.g. not yet created): hash the raw path.
}
// Everything below validates a landing this process may have ADOPTED
// rather than created. The path is fully predictable — the project hash
// is a pure function of the root — and 0700 does not exclude the user's
// own other processes, so "it exists already" is not evidence that this
// tool made it. The landing is where relocation puts artifacts precisely
// BECAUSE they must stay private, so adoption is validated, not assumed.
//
// Validation walks EVERY component this method creates, not just the
// leaf. `mkdirSync(…, { recursive: true })` follows symlinks in every
// component above the final one, and `lstat` refuses to follow only the
// final one — so a leaf-only check cannot see a redirected parent. With
// `audits` planted as a symlink (one `ln -s`, no race: `~/.qwen` exists
// long before `audits` does), the leaf is created inside the planter's
// directory, reports as a perfectly real directory, and every artifact
// written "into the landing" lands wherever the link points. Binding
// writes to "this root" later cannot help if the root itself moved.
//
// QWEN_HOME itself is deliberately NOT validated here: it is the user's
// own configured location, not a path this method invents.
const auditsDir = Storage.adoptDirectory(
path.join(Storage.getGlobalQwenDir(), 'audits'),
'the audit artifact directory',
);
const dir = Storage.adoptDirectory(
path.join(auditsDir, getProjectHash(resolved)),
'the fallback landing',
);
Storage.assertAuditLandingIsClean(dir);
return dir;
}

/**
* Create one path component and return it only if what is there now is a
* real directory owned by this user's private mode.
*
* Non-recursive on purpose: `recursive: true` would silently walk (and
* follow) anything already standing in the path. Creating exactly one
* component at a time is what makes each component checkable.
*/
private static adoptDirectory(dir: string, what: string): string {
try {
fs.mkdirSync(dir, { mode: 0o700 });
} catch (err) {
// EEXIST is the adoption case the checks below exist for. Anything
// else (a missing parent, a permission error) surfaces as itself.
if ((err as NodeJS.ErrnoException).code !== 'EEXIST') throw err;
}
const stat = fs.lstatSync(dir);
if (!stat.isDirectory()) {
throw new Error(
`audit: ${what} ${dir} is not a directory (it may be a symlink ` +
`planted ahead of the run) — remove it and re-run.`,
);
}
// mkdirSync's mode applies only to directories it CREATES, so a
// pre-existing component keeps whatever mode it was given. Windows
// reports a mode that does not carry POSIX group/other bits; chmod there
// is a no-op, and the check would fire on every run.
if (process.platform !== 'win32' && (stat.mode & 0o077) !== 0) {
fs.chmodSync(dir, 0o700);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Declined as proposed, with the limitation documented instead. Refusing adoption of pre-existing components on win32 would also refuse this tool's OWN landings from previous runs — the landing is deliberately reused across runs (the report and its sidecar are the durable artifacts), and on Windows nothing distinguishes self-created from planted. The effect would be every second /audit run per project failing forever with "remove it and re-run", breaking the reuse contract this same file argues for. Instead the mode-repair comment now states the win32 truth plainly: an adopted component keeps whatever DACL it had, since Node exposes no portable ACL enforcement. If a maintainer wants a stronger win32 guarantee anyway, a creation marker (written when this tool creates a component, required for adoption) could distinguish self-created landings — say the word and I will implement it.

中文说明

按提议的方案予以拒绝,改为如实记录该限制。在 win32 上拒绝采纳已存在组件,会同时拒绝本工具此前运行自建的落点——落点是刻意跨运行复用的(报告及其 sidecar 是持久产物),而 Windows 上无法区分自建与植入。其后果将是每个项目每隔一次 /audit 运行就永远失败并提示「移除后重跑」,打破同一文件为之辩护的复用契约。因此改为在模式修复注释中如实写明 win32 现状:已存在组件保留其既有 DACL,因为 Node 没有可移植的 ACL 强制。若维护者仍希望更强的 win32 保证,可以用创建标记(本工具创建组件时写入、采纳时要求存在)来区分自建落点——如有指示,乐意实现。

}
return dir;
}

/**
* Refuse a fallback landing whose CONTENTS would redirect writes out of it.
*
* Validating the leaf alone is not enough: artifacts land at paths BELOW it
* (`audit-<ts>.sidecar/sidecar.json`, the dated report), and an
* O_NOFOLLOW open only ever guards the final component. A planted symlink
* child is therefore a complete escape — `mkdirSync` happily treats a
* symlink-to-directory as the directory, and every artifact written
* "inside" the landing lands wherever the link points, while the leaf
* itself stays a perfectly valid directory that re-validation passes.
* A hardlinked regular file is the same story for reads: an existing name
* reopened with O_TRUNC writes into the planter's inode.
*
* The landing is REUSED across runs (the report and its sidecar are the
* durable artifacts), so this cannot refuse a non-empty landing — only
* entries that are not what a previous run of this tool would have left.
*/
private static assertAuditLandingIsClean(dir: string): void {
let entries: fs.Dirent[];
try {
entries = fs.readdirSync(dir, { withFileTypes: true });
} catch {
// Unreadable right after a successful lstat: nothing can be validated,
// and nothing can be written either — let the write surface it.
return;
}
for (const entry of entries) {
// Some filesystems return an unknown dirent type, and then BOTH
// isSymbolicLink() and isFile() answer false — the entry would slip
// past every arm below. Ask the kernel directly for those.
const typed =
entry.isSymbolicLink() || entry.isFile() || entry.isDirectory();
let stat: fs.Stats | undefined;
if (!typed) {
try {
stat = fs.lstatSync(path.join(dir, entry.name));
} catch {
continue; // vanished between readdir and lstat
}
}
if (entry.isSymbolicLink() || stat?.isSymbolicLink()) {
throw new Error(
`audit: the fallback landing ${dir} contains a symlink ` +
`(${entry.name}) — artifacts written under it would land outside ` +
`the landing. Remove it and re-run.`,
);
}
if (!(entry.isFile() || stat?.isFile())) continue;
// A hardlink twin proves another name for the same inode exists
// somewhere this check can never see.
let links: number;
try {
links = (stat ?? fs.lstatSync(path.join(dir, entry.name))).nlink;
} catch {
continue; // vanished between readdir and lstat
}
if (links > 1) {
throw new Error(
`audit: the fallback landing ${dir} contains a hardlinked file ` +
`(${entry.name}) — a write to it would also write through its ` +
`twin. Remove it and re-run.`,
);
}
}
}

getQwenDir(): string {
return path.join(this.targetDir, QWEN_DIR);
}
Expand Down
Loading