fix: add onCompromised handlers to proper-lockfile calls to prevent daemon crash (#8442)

* fix: add onCompromised handlers to proper-lockfile calls to prevent daemon crash

When a lock.lock directory is deleted while held (e.g. by another
process cleaning stale locks), proper-lockfile's updateLock timer
gets ENOENT on stat and calls the default onCompromised handler,
which throws and crashes the process.

Add onCompromised handlers that log instead of throw, consistent
with existing handlers in tasks.ts and mailbox.ts.

* fix: guard lock release after compromise and cover trusted folders (#8442)

* fix: guard mailbox/task lock release and cover compromise handlers with tests (#8442)

* test(core): add lock-compromise regression tests for mailbox and task guards (#8442)

* test: share lock-compromise simulation via test-utils helpers (#8442)

---------

Co-authored-by: 易良 <1204183885@qq.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
This commit is contained in:
Shaojin Wen 2026-08-05 11:05:28 +08:00 committed by GitHub
parent 3084326243
commit 8b0e8b8192
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 281 additions and 6 deletions

View file

@ -23,6 +23,7 @@ import {
import * as fs from 'node:fs';
import stripJsonComments from 'strip-json-comments';
import * as path from 'node:path';
import lockfile from 'proper-lockfile';
import * as jsoncEditor from '../utils/jsonc-editor.js';
import {
loadTrustedFolders,
@ -335,6 +336,29 @@ describe('Trusted Folders Loading', () => {
expect(writtenContent).toContain('"/new/path": "TRUST_FOLDER"');
});
it('saveTrustedFolders registers a lock-compromised handler', () => {
const userPath = getTrustedFoldersPath();
const dirPath = path.dirname(userPath);
(mockFsExistsSync as Mock).mockImplementation(
(p) => p === userPath || p === dirPath,
);
(fs.readFileSync as Mock).mockReturnValue('{}');
saveTrustedFolders({
path: userPath,
config: {
'/new/path': TrustLevel.TRUST_FOLDER,
},
});
const options = vi.mocked(lockfile.lockSync).mock.calls.at(-1)?.[1];
expect(options?.onCompromised).toBeTypeOf('function');
expect(() =>
options?.onCompromised?.(new Error('lock lost')),
).not.toThrow();
});
it('saveTrustedFolders should reject malformed input without overwriting it', () => {
const userPath = getTrustedFoldersPath();
const dirPath = path.dirname(userPath);

View file

@ -9,6 +9,7 @@ import * as path from 'node:path';
import lockfile from 'proper-lockfile';
import {
atomicWriteFileSync,
createDebugLogger,
FatalConfigError,
getErrorMessage,
ideContextStore,
@ -23,6 +24,8 @@ import {
isWithinRoot,
} from './path-comparison.js';
const debugLogger = createDebugLogger('TRUSTED_FOLDERS');
export const TRUSTED_FOLDERS_FILENAME = 'trustedFolders.json';
export function getTrustedFoldersPath(): string {
@ -255,6 +258,9 @@ function writeTrustedFolders(
const release = lockfile.lockSync(filePath, {
realpath: false,
stale: 10_000,
onCompromised: (err) => {
debugLogger.warn('trusted folders lock compromised:', err);
},
});
try {
let originalContent = '{}';

View file

@ -0,0 +1,39 @@
/**
* @license
* Copyright 2026 Qwen Team
* SPDX-License-Identifier: Apache-2.0
*/
import lockfile from 'proper-lockfile';
import { vi } from 'vitest';
/**
* Simulates a proper-lockfile compromise on the next `lockfile.lock` call.
* After a real compromise, proper-lockfile marks the lock released before
* invoking `onCompromised`, so the later `release()` rejects with
* `ERELEASED`; the mocked release reproduces that exact state.
*/
export function mockCompromisedLock() {
let onCompromised: ((error: Error) => void) | undefined;
let lockedFile: string | undefined;
const lockSpy = vi
.spyOn(lockfile, 'lock')
.mockImplementationOnce(async (file, options) => {
lockedFile = file;
onCompromised = options?.onCompromised;
onCompromised?.(
Object.assign(new Error('lock lost'), { code: 'ECOMPROMISED' }),
);
return () =>
Promise.reject(
Object.assign(new Error('Lock is already released'), {
code: 'ERELEASED',
}),
);
});
return {
lockSpy,
getLockedFile: () => lockedFile,
getOnCompromised: () => onCompromised,
};
}

View file

@ -17,6 +17,7 @@ import {
import { EventEmitter } from 'node:events';
import type { spawn } from 'node:child_process';
import { createHash } from 'node:crypto';
import { mockCompromisedLock } from '../test-utils/mock-compromised-lock.js';
const temporaryDirectories: string[] = [];
@ -130,6 +131,36 @@ describe('managed npm update', () => {
expect(fs.readFileSync(runningChunk, 'utf8')).toBe('old chunk');
});
it('still activates when the lock is compromised during activation', async () => {
const root = makeTemporaryDirectory();
const bootstrap = writeBaseInstallation(root);
const update = prepareManagedNpmUpdate(
'2.0.0',
bootstrap,
path.join(root, 'updates'),
);
writeInstallation(update.stagingDir, '2.0.0');
const { lockSpy, getOnCompromised } = mockCompromisedLock();
try {
await expect(
activateManagedNpmUpdate(update, '2.0.0', bootstrap),
).resolves.toBeUndefined();
expect(getOnCompromised()).toBeTypeOf('function');
expect(
JSON.parse(
fs.readFileSync(
path.join(update.launcherRoot, 'active.json'),
'utf8',
),
),
).toMatchObject({ version: '2.0.0' });
} finally {
lockSpy.mockRestore();
}
});
it('installs and activates inside the managed worker', async () => {
const root = makeTemporaryDirectory();
const bootstrap = writeBaseInstallation(root);

View file

@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { Storage } from '@qwen-code/qwen-code-core';
import { Storage, createDebugLogger } from '@qwen-code/qwen-code-core';
import { execFile, execFileSync, spawn } from 'node:child_process';
import { createHash } from 'node:crypto';
import * as fs from 'node:fs';
@ -16,6 +16,7 @@ import semver from 'semver';
import { getNpmCliPath } from './installationInfo.js';
const PACKAGE_NAME = '@qwen-code/qwen-code';
const debugLogger = createDebugLogger('MANAGED_NPM_UPDATE');
const execFileAsync = promisify(execFile);
interface ManagedNpmUpdate {
@ -310,6 +311,9 @@ export async function activateManagedNpmUpdate(
realpath: false,
stale: 30_000,
retries: { retries: 50, minTimeout: 20, maxTimeout: 100 },
onCompromised: (err) => {
debugLogger.warn('managed npm update lock compromised:', err);
},
});
try {
const base = readBaseInstallation(resolvedBootstrapPath);
@ -365,7 +369,11 @@ export async function activateManagedNpmUpdate(
await fsPromises.rm(temporaryActivePath, { force: true });
}
} finally {
await release();
try {
await release();
} catch (error) {
debugLogger.warn('Failed to release managed npm update lock:', error);
}
}
}

View file

@ -19,6 +19,7 @@ import {
sendStructuredMessage,
disposeInboxLocks,
} from './mailbox.js';
import { mockCompromisedLock } from '../../test-utils/mock-compromised-lock.js';
vi.mock('../../config/storage.js', async (importOriginal) => {
const original =
@ -139,6 +140,24 @@ describe('mailbox', () => {
expect(texts).toContain('fresh');
});
// ─── Lock compromise ─────────────────────────────────────
it('still writes the message when the lock is compromised', async () => {
const { lockSpy, getOnCompromised } = mockCompromisedLock();
try {
await expect(
writeMessage('team', 'worker', makeMessage({ text: 'compromised' })),
).resolves.toBeUndefined();
expect(getOnCompromised()).toBeTypeOf('function');
} finally {
lockSpy.mockRestore();
}
const messages = await readInbox('team', 'worker');
expect(messages.map((m) => m.text)).toEqual(['compromised']);
});
// ─── consumeUnread ─────────────────────────────────────────
it('returns unread messages and marks them read', async () => {

View file

@ -83,7 +83,7 @@ const LOCK_OPTIONS: lockfile.LockOptions = {
// Stale locks from crashed processes are expected in multi-agent
// scenarios; log at debug level for traceability without noise.
onCompromised: (err) => {
debug.debug('mailbox lock compromised:', err?.message ?? err);
debug.debug('mailbox lock compromised:', err);
},
};
@ -150,7 +150,11 @@ async function withInboxLock<T>(
try {
return await fn();
} finally {
await release();
try {
await release();
} catch (error) {
debug.debug('Failed to release mailbox lock:', error);
}
}
});
}

View file

@ -25,6 +25,7 @@ import {
TaskOwnershipError,
RECIPROCAL_CALLER,
} from './tasks.js';
import { mockCompromisedLock } from '../../test-utils/mock-compromised-lock.js';
vi.mock('../../config/storage.js', async (importOriginal) => {
const original =
@ -153,6 +154,27 @@ describe('tasks', () => {
expect(updated!.owner).toBe('worker@team');
});
it('still updates the task when the lock is compromised', async () => {
const task = await createTask('team', {
subject: 'Test',
description: 'Desc',
});
const { lockSpy, getOnCompromised } = mockCompromisedLock();
try {
await expect(
updateTask('team', task.id, { status: 'in_progress' }),
).resolves.toMatchObject({ id: task.id, status: 'in_progress' });
expect(getOnCompromised()).toBeTypeOf('function');
} finally {
lockSpy.mockRestore();
}
expect(await getTask('team', task.id)).toMatchObject({
status: 'in_progress',
});
});
it('clears owner with null', async () => {
const task = await createTask('team', {
subject: 'Test',

View file

@ -79,7 +79,7 @@ const LOCK_OPTIONS: lockfile.LockOptions = {
},
stale: 5000,
onCompromised: (err) => {
debug.warn('task lock compromised:', err?.message ?? err);
debug.warn('task lock compromised:', err);
},
};
@ -132,7 +132,11 @@ async function withTaskFileLock<T>(
try {
return await fn();
} finally {
await release();
try {
await release();
} catch (error) {
debug.warn('Failed to release task lock:', error);
}
}
});
}

View file

@ -16,6 +16,7 @@ import {
ExtensionStore,
ExtensionStoreCorruptError,
} from './extension-store.js';
import { mockCompromisedLock } from '../test-utils/mock-compromised-lock.js';
describe('ExtensionStore', () => {
let root: string;
@ -269,6 +270,21 @@ describe('ExtensionStore', () => {
});
});
it('registers a lock-compromised handler and completes when the store lock is compromised', async () => {
const store = makeStore();
const identity = { id: 'd4'.repeat(32), name: 'demo' };
const { lockSpy, getOnCompromised } = mockCompromisedLock();
try {
await expect(store.ensureInitialized([identity])).resolves.toMatchObject({
generation: 0,
});
expect(getOnCompromised()).toBeTypeOf('function');
} finally {
lockSpy.mockRestore();
}
});
it('serializes mutations from two Node processes sharing QWEN_HOME', async () => {
const id = 'd2'.repeat(32);
const store = makeStore();

View file

@ -1014,6 +1014,9 @@ export class ExtensionStore {
maxTimeout: 500,
randomize: true,
},
onCompromised: (err) => {
debugLogger.warn('extension store lock compromised:', err);
},
});
} catch (error) {
throw new ExtensionStoreBusyError(this.storeDir, { cause: error });

View file

@ -55,6 +55,8 @@ const fsFailure = vi.hoisted(() => ({
const lockObservation = vi.hoisted(() => ({
path: undefined as string | undefined,
attempted: undefined as (() => void) | undefined,
options: undefined as lockfile.LockOptions | undefined,
simulateCompromise: false,
}));
vi.mock('proper-lockfile', async (importOriginal) => {
@ -65,7 +67,17 @@ vi.mock('proper-lockfile', async (importOriginal) => {
...actual,
async lock(...args: Parameters<typeof actual.lock>) {
if (String(args[0]) === lockObservation.path) {
lockObservation.options = args[1];
lockObservation.attempted?.();
const release = await actual.lock(...args);
if (lockObservation.simulateCompromise) {
// A real compromise marks the lock released in the registry
// before invoking onCompromised, so release() later rejects
// with ERELEASED. Reproduce that state exactly.
lockObservation.options?.onCompromised?.(new Error('lock lost'));
await release();
}
return release;
}
return actual.lock(...args);
},
@ -164,6 +176,8 @@ describe('channel memory', () => {
fsFailure.legacyAppendAfterRename = undefined;
lockObservation.path = undefined;
lockObservation.attempted = undefined;
lockObservation.options = undefined;
lockObservation.simulateCompromise = false;
vi.restoreAllMocks();
if (originalQwenHome === undefined) {
delete process.env['QWEN_HOME'];
@ -437,6 +451,22 @@ describe('channel memory', () => {
}
});
it('completes mutations when the channel memory lock is compromised', async () => {
writeJson(serializeChannelMemoryDocument({ version: 1, entries: [] }));
lockObservation.path = path.join(
path.dirname(getChannelMemoryFilePath(target)),
'.channel-memory.lock',
);
lockObservation.simulateCompromise = true;
await expect(
addChannelMemoryEntries(target, ['Run tests']),
).resolves.toMatchObject({ changed: true });
expect(lockObservation.options?.onCompromised).toBeTypeOf('function');
await expect(readChannelMemory(target)).resolves.toBe('Run tests\n');
});
it('does not delete legacy bytes changed after canonical commit', async () => {
const legacyPath = writeLegacy('Use staging\n');
fsFailure.legacyAppendAfterRename = {

View file

@ -9,6 +9,7 @@ import * as fs from 'node:fs/promises';
import * as path from 'node:path';
import lockfile from 'proper-lockfile';
import { Storage } from '../config/storage.js';
import { createDebugLogger } from '../utils/debugLogger.js';
import {
createChannelMemoryEntry,
MAX_CHANNEL_MEMORY_ENTRIES_PER_REQUEST,
@ -52,6 +53,8 @@ export const CHANNEL_MEMORY_FILE_NAME = 'CHANNEL.json';
export const LEGACY_CHANNEL_MEMORY_FILE_NAME = 'CHANNEL.md';
export const MAX_CHANNEL_MEMORY_BYTES = 1024 * 1024;
const debugLogger = createDebugLogger('CHANNEL_MEMORY');
const pendingMutations = new Map<string, Promise<void>>();
const LOCK_OPTIONS: lockfile.LockOptions = {
realpath: false,
@ -63,6 +66,9 @@ const LOCK_OPTIONS: lockfile.LockOptions = {
randomize: true,
},
stale: 5000,
onCompromised: (err) => {
debugLogger.warn('channel memory lock compromised:', err);
},
};
interface LoadedChannelMemory {

View file

@ -17,6 +17,7 @@ import {
runAutoSkillCurator,
setAutoSkillPinned,
} from './skill-curator.js';
import { mockCompromisedLock } from '../test-utils/mock-compromised-lock.js';
const DAY_MS = 24 * 60 * 60 * 1000;
@ -91,6 +92,23 @@ describe('auto-skill curator', () => {
).rejects.toMatchObject({ code: 'ENOENT' });
});
it('registers a lock-compromised handler and completes when the curator lock is compromised', async () => {
const now = new Date('2026-07-27T00:00:00.000Z');
const { lockSpy, getLockedFile, getOnCompromised } = mockCompromisedLock();
try {
await expect(
runAutoSkillCurator(projectRoot, { now }),
).resolves.toMatchObject({ dryRun: false });
expect(getOnCompromised()).toBeTypeOf('function');
expect(getLockedFile()).toBe(
path.join(projectRoot, '.qwen', 'skill-curator.lock'),
);
} finally {
lockSpy.mockRestore();
}
});
it('ignores auto-skill directories whose names carry control/ANSI bytes', async () => {
const now = new Date('2026-07-27T00:00:00.000Z');
const old = new Date(now.getTime() - 100 * DAY_MS);

View file

@ -10,6 +10,7 @@ import * as path from 'node:path';
import lockfile from 'proper-lockfile';
import { QWEN_DIR } from '../config/storage.js';
import { atomicWriteJSON } from '../utils/atomicFileWrite.js';
import { createDebugLogger } from '../utils/debugLogger.js';
import { parse as parseYaml } from '../utils/yaml-parser.js';
import {
getArchivedSkillsRoot,
@ -23,6 +24,8 @@ export const AUTO_SKILL_CURATOR_INTERVAL_MS = 7 * 24 * 60 * 60 * 1000;
export const AUTO_SKILL_STALE_AFTER_MS = 30 * 24 * 60 * 60 * 1000;
export const AUTO_SKILL_ARCHIVE_AFTER_MS = 90 * 24 * 60 * 60 * 1000;
const debugLogger = createDebugLogger('SKILL_CURATOR');
const AUTO_SKILL_PREFIX = 'auto-skill-';
const CURATOR_STATE_VERSION = 1;
const CURATOR_STATE_FILE = 'skill-curator.json';
@ -156,6 +159,9 @@ const LOCK_OPTIONS: lockfile.LockOptions = {
randomize: true,
},
stale: 10_000,
onCompromised: (err) => {
debugLogger.warn('skill curator lock compromised:', err);
},
};
function getCuratorPaths(projectRoot: string): CuratorPaths {

View file

@ -0,0 +1,39 @@
/**
* @license
* Copyright 2026 Qwen Team
* SPDX-License-Identifier: Apache-2.0
*/
import lockfile from 'proper-lockfile';
import { vi } from 'vitest';
/**
* Simulates a proper-lockfile compromise on the next `lockfile.lock` call.
* After a real compromise, proper-lockfile marks the lock released before
* invoking `onCompromised`, so the later `release()` rejects with
* `ERELEASED`; the mocked release reproduces that exact state.
*/
export function mockCompromisedLock() {
let onCompromised: ((error: Error) => void) | undefined;
let lockedFile: string | undefined;
const lockSpy = vi
.spyOn(lockfile, 'lock')
.mockImplementationOnce(async (file, options) => {
lockedFile = file;
onCompromised = options?.onCompromised;
onCompromised?.(
Object.assign(new Error('lock lost'), { code: 'ECOMPROMISED' }),
);
return () =>
Promise.reject(
Object.assign(new Error('Lock is already released'), {
code: 'ERELEASED',
}),
);
});
return {
lockSpy,
getLockedFile: () => lockedFile,
getOnCompromised: () => onCompromised,
};
}