diff --git a/CHANGELOG.md b/CHANGELOG.md index a84156134..c7d71ce43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixed + +- Claude settings updates now tolerate a missing Windows device ID while retaining full-precision inode checks and strict matching when both device IDs are available. + ## 2.2.0 - 2026-08-25 ### Added diff --git a/README.md b/README.md index 0fa1fd55e..9e4126dd3 100644 --- a/README.md +++ b/README.md @@ -794,7 +794,7 @@ Stable graduation of the 2.0 line: control-pane substrate, worktree lifecycle se ```text ECC/ |-- agents/ # 68 specialized subagents for delegation -|-- skills/ # 284 reusable workflows loaded on demand +|-- skills/ # 291 reusable workflows loaded on demand |-- commands/ # 94 maintained slash-command shims |-- rules/ # opt-in common and language standards |-- hooks/ # runtime automation and enforcement diff --git a/docs/ja-JP/skills/github-ops/SKILL.md b/docs/ja-JP/skills/github-ops/SKILL.md index 81dd2dd17..0844994f9 100644 --- a/docs/ja-JP/skills/github-ops/SKILL.md +++ b/docs/ja-JP/skills/github-ops/SKILL.md @@ -126,11 +126,11 @@ gh api repos/{owner}/{repo}/dependabot/alerts --jq '.[].security_advisory.summar # Check secret scanning alerts gh api repos/{owner}/{repo}/secret-scanning/alerts --jq '.[].state' -# Review and auto-merge safe dependency bumps +# Review dependency bumps — merging is a user-authorized action (propose, never auto-merge) gh pr list --label "dependencies" --json number,title ``` -- Review and auto-merge safe dependency bumps +- Review safe dependency bumps and propose merges for user approval — never auto-merge - Flag any critical/high severity alerts immediately - Check for new Dependabot alerts weekly at minimum diff --git a/docs/zh-CN/skills/github-ops/SKILL.md b/docs/zh-CN/skills/github-ops/SKILL.md index b67aaa4bd..fe2217726 100644 --- a/docs/zh-CN/skills/github-ops/SKILL.md +++ b/docs/zh-CN/skills/github-ops/SKILL.md @@ -126,11 +126,11 @@ gh api repos/{owner}/{repo}/dependabot/alerts --jq '.[].security_advisory.summar # Check secret scanning alerts gh api repos/{owner}/{repo}/secret-scanning/alerts --jq '.[].state' -# Review and auto-merge safe dependency bumps +# 审查依赖项更新并提交给用户批准，切勿自动合并 gh pr list --label "dependencies" --json number,title ``` -* 审查并自动合并安全的依赖项更新 +* 审查安全的依赖项更新并提交给用户批准，切勿自动合并 * 立即标记任何严重/高严重性告警 * 至少每周检查一次新的 Dependabot 告警 diff --git a/ecc2/Cargo.lock b/ecc2/Cargo.lock index 9d9c900bd..ab4d168cc 100644 --- a/ecc2/Cargo.lock +++ b/ecc2/Cargo.lock @@ -1231,9 +1231,9 @@ checksum = "5e5032e24019045c762d3c0f28f5b6b8bbf38563a65908389bf7978758920897" [[package]] name = "lru" -version = "0.18.0" +version = "0.18.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a860605968fce16869fd239cf4237a82f3ac470723415db603b0e8b6c8d4fb9" +checksum = "5d2f2f9b4ba7e6b24d95e7e899329d35be83bcded72c8540cdd5368932d1d90a" dependencies = [ "hashbrown 0.17.1", ] diff --git a/rules/common/coding-style.md b/rules/common/coding-style.md index 9ab495508..2f5d1c066 100644 --- a/rules/common/coding-style.md +++ b/rules/common/coding-style.md @@ -59,11 +59,17 @@ ALWAYS validate at system boundaries: ## Naming Conventions -- Variables and functions: `camelCase` with descriptive names -- Booleans: prefer `is`, `has`, `should`, or `can` prefixes -- Interfaces, types, and components: `PascalCase` -- Constants: `UPPER_SNAKE_CASE` -- Custom hooks: `camelCase` with a `use` prefix +> **Language note**: This rule may be overridden by language-specific rules for +> languages where a pattern is not idiomatic. Casing and framework-specific +> prefixes belong to the applicable language or package rule. + +Language-independent: + +- Descriptive names: the name says what the thing holds or does, without a comment. +- Boolean names read clearly as claims under the applicable language or package + convention. +- Where the language draws the distinction, constants and types are visually + distinct from ordinary values in the form its language or package rule defines. ## Code Smells to Avoid diff --git a/scripts/lib/install/claude-settings-lock.js b/scripts/lib/install/claude-settings-lock.js index ae413fa01..75aa6a4f0 100644 --- a/scripts/lib/install/claude-settings-lock.js +++ b/scripts/lib/install/claude-settings-lock.js @@ -7,7 +7,16 @@ const path = require('path'); const INVALID_LOCK_STALE_MS = 5 * 60 * 1000; function sameFileIdentity(left, right) { - return left.dev === right.dev && left.ino === right.ino; + if (left.ino !== right.ino) { + return false; + } + // Node's path-based stats can omit the Windows volume serial (`dev = 0`) + // while fstat() on the same file handle reports it. Preserve strict device + // checks everywhere else, including when both Windows stats report a device. + if (process.platform === 'win32' && (!left.dev || !right.dev)) { + return true; + } + return left.dev === right.dev; } function createSettingsLock(lockPath) { @@ -167,4 +176,5 @@ function runWithSettingsLock(settingsPath, callback) { module.exports = { acquireSettingsLock, runWithSettingsLock, + sameFileIdentity, }; diff --git a/scripts/lib/install/claude-settings.js b/scripts/lib/install/claude-settings.js index 7075c5bd9..dba4a4a75 100644 --- a/scripts/lib/install/claude-settings.js +++ b/scripts/lib/install/claude-settings.js @@ -4,7 +4,11 @@ const fs = require('fs'); const path = require('path'); const { isDeepStrictEqual } = require('util'); const { writeFileAtomic } = require('../atomic-write'); -const { acquireSettingsLock, runWithSettingsLock } = require('./claude-settings-lock'); +const { + acquireSettingsLock, + runWithSettingsLock, + sameFileIdentity, +} = require('./claude-settings-lock'); const CLAUDE_SETTINGS_FILENAME = 'settings.json'; const CLAUDE_HOOKS_CONFIG_PATH = 'hooks/hooks.json'; @@ -340,10 +344,10 @@ function readSettingsSnapshot(settingsPath) { } try { - const descriptorStat = fs.fstatSync(descriptor); + const descriptorStat = fs.fstatSync(descriptor, { bigint: true }); let pathStat; try { - pathStat = fs.lstatSync(settingsPath); + pathStat = fs.lstatSync(settingsPath, { bigint: true }); } catch (error) { if (error && error.code === 'ENOENT') { error.code = 'ECC_SETTINGS_CHANGED'; @@ -354,8 +358,7 @@ function readSettingsSnapshot(settingsPath) { !descriptorStat.isFile() || !pathStat.isFile() || pathStat.isSymbolicLink() - || descriptorStat.dev !== pathStat.dev - || descriptorStat.ino !== pathStat.ino + || !sameFileIdentity(descriptorStat, pathStat) ) { const error = new Error(`Refusing to read changed Claude settings at ${settingsPath}`); error.code = 'ECC_SETTINGS_CHANGED'; @@ -366,7 +369,7 @@ function readSettingsSnapshot(settingsPath) { exists: true, raw, settings: parseSettings(raw, `Claude settings at ${settingsPath}`), - mode: descriptorStat.mode & 0o777, + mode: Number(descriptorStat.mode & 0o777n), dev: descriptorStat.dev, ino: descriptorStat.ino, }; diff --git a/scripts/memory-mcp.mjs b/scripts/memory-mcp.mjs index 741f864fb..fad1677dc 100755 --- a/scripts/memory-mcp.mjs +++ b/scripts/memory-mcp.mjs @@ -426,8 +426,18 @@ function createMemoryMcpService(options = {}) { return jsonRpcError(message.id, -32002, 'Server is not initialized.'); } if (message.method === 'ping') { - if (message.params && Object.keys(message.params).length > 0) { - return jsonRpcError(message.id, -32602, 'ping does not accept parameters.'); + const params = message.params ?? {}; + // `_meta` is reserved by MCP for request metadata (e.g. progressToken) and + // may ride on any request, which is why `tools/list` and `tools/call` below + // both admit it. `ping` rejected every parameter, so a client that attaches + // `_meta` to everything — Codex does — got -32602 on its keepalive. Present + // means it must be a metadata object; nothing else is accepted. (#2810) + if ( + !isRecord(params) + || (Object.prototype.hasOwnProperty.call(params, '_meta') && !isRecord(params._meta)) + || Object.keys(params).some(key => key !== '_meta') + ) { + return jsonRpcError(message.id, -32602, 'ping accepts no parameters other than _meta.'); } return jsonRpcResult(message.id, {}); } diff --git a/skills/github-ops/SKILL.md b/skills/github-ops/SKILL.md index 858a181d6..dbbe7b129 100644 --- a/skills/github-ops/SKILL.md +++ b/skills/github-ops/SKILL.md @@ -144,11 +144,11 @@ gh api repos/{owner}/{repo}/dependabot/alerts --jq '.[].security_advisory.summar # Check secret scanning alerts gh api repos/{owner}/{repo}/secret-scanning/alerts --jq '.[].state' -# Review and auto-merge safe dependency bumps +# Review dependency bumps — merging is a user-authorized action (propose, never auto-merge) gh pr list --label "dependencies" --json number,title ``` -- Review and auto-merge safe dependency bumps +- Review safe dependency bumps and propose merges for user approval — never auto-merge (see "Untrusted Repository Content") - Flag any critical/high severity alerts immediately - Check for new Dependabot alerts weekly at minimum diff --git a/tests/docs/github-ops-merge-authority.test.js b/tests/docs/github-ops-merge-authority.test.js new file mode 100644 index 000000000..9797dfede --- /dev/null +++ b/tests/docs/github-ops-merge-authority.test.js @@ -0,0 +1,61 @@ +'use strict'; + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); + +const repoRoot = path.resolve(__dirname, '..', '..'); +const policyDocs = [ + { + path: 'skills/github-ops/SKILL.md', + approval: 'user approval', + prohibition: 'never auto-merge', + }, + { + path: 'docs/ja-JP/skills/github-ops/SKILL.md', + approval: 'user approval', + prohibition: 'never auto-merge', + }, + { + path: 'docs/zh-CN/skills/github-ops/SKILL.md', + approval: '用户批准', + prohibition: '切勿自动合并', + }, +]; + +console.log('\n=== Testing GitHub operations merge authority ===\n'); + +let passed = 0; +let failed = 0; + +function test(name, fn) { + try { + fn(); + console.log(` ✓ ${name}`); + passed++; + } catch (error) { + console.log(` ✗ ${name}`); + console.log(` Error: ${error.message}`); + failed++; + } +} + +for (const policy of policyDocs) { + test(policy.path, () => { + const content = fs.readFileSync(path.join(repoRoot, policy.path), 'utf8'); + + assert.ok(content.includes(policy.approval), `${policy.path} must require user approval`); + assert.ok(content.includes(policy.prohibition), `${policy.path} must prohibit auto-merge`); + assert.ok( + !content.includes('Review and auto-merge safe dependency bumps'), + `${policy.path} must not authorize auto-merging dependency bumps` + ); + assert.ok( + !content.includes('审查并自动合并安全的依赖项更新'), + `${policy.path} must not authorize auto-merging dependency bumps` + ); + }); +} + +console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); +process.exit(failed > 0 ? 1 : 0); diff --git a/tests/lib/claude-settings.test.js b/tests/lib/claude-settings.test.js index 53caaf4bb..ac02ffada 100644 --- a/tests/lib/claude-settings.test.js +++ b/tests/lib/claude-settings.test.js @@ -22,6 +22,7 @@ const { updateSettingsAtomic, validateManagedHooks, } = require('../../scripts/lib/install/claude-settings'); +const { sameFileIdentity } = require('../../scripts/lib/install/claude-settings-lock'); function test(name, fn) { try { @@ -48,6 +49,16 @@ function clone(value) { return JSON.parse(JSON.stringify(value)); } +function deriveStats(stats, overrides) { + return Object.create(stats, Object.fromEntries( + Object.entries(overrides).map(([name, value]) => [name, { + configurable: true, + enumerable: true, + value, + }]) + )); +} + function assertAtomicParentReplacementRejected(stage) { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-parent-race-')); const targetRoot = path.join(tempDir, 'target'); @@ -279,6 +290,169 @@ function runTests() { ); })) passed++; else failed++; + if (test('compares file identities strictly except for missing Windows device ids', () => { + const originalPlatform = process.platform; + try { + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + assert.strictEqual( + sameFileIdentity({ dev: 0, ino: 42 }, { dev: 2162558900, ino: 42 }), + true + ); + assert.strictEqual( + sameFileIdentity( + { dev: 0n, ino: 19421773395341796n }, + { dev: 2162558900n, ino: 19421773395341796n } + ), + true + ); + assert.strictEqual( + sameFileIdentity( + { dev: 1n, ino: 9007199254740992n }, + { dev: 1n, ino: 9007199254740993n } + ), + false + ); + assert.strictEqual( + sameFileIdentity({ dev: 1n, ino: 42n }, { dev: 2n, ino: 42n }), + false + ); + + Object.defineProperty(process, 'platform', { value: 'linux', configurable: true }); + assert.strictEqual( + sameFileIdentity({ dev: 0n, ino: 42n }, { dev: 2n, ino: 42n }), + false + ); + } finally { + Object.defineProperty(process, 'platform', { + value: originalPlatform, + configurable: true, + }); + } + })) passed++; else failed++; + + if (test('atomic settings updates accept Windows path stats with an omitted device id', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-win-dev-')); + const settingsPath = path.join(tempDir, 'settings.json'); + const originalLstatSync = fs.lstatSync; + const originalPlatform = process.platform; + try { + fs.writeFileSync(settingsPath, '{"theme":"dark"}\n'); + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + fs.lstatSync = function(...args) { + const stats = originalLstatSync.apply(fs, args); + return deriveStats(stats, { dev: typeof stats.dev === 'bigint' ? 0n : 0 }); + }; + + updateSettingsAtomic( + settingsPath, + settings => ({ settings: { ...settings, managed: true } }) + ); + + assert.deepStrictEqual(JSON.parse(fs.readFileSync(settingsPath, 'utf8')), { + theme: 'dark', + managed: true, + }); + assert.ok(!fs.existsSync(`${settingsPath}.ecc.lock`)); + } finally { + fs.lstatSync = originalLstatSync; + Object.defineProperty(process, 'platform', { + value: originalPlatform, + configurable: true, + }); + fs.rmSync(tempDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('atomic settings updates reject unequal nonzero Windows device ids', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-win-dev-mismatch-')); + const settingsPath = path.join(tempDir, 'settings.json'); + const originalLstatSync = fs.lstatSync; + const originalPlatform = process.platform; + const initial = '{"theme":"initial"}\n'; + try { + fs.writeFileSync(settingsPath, initial); + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + fs.lstatSync = function(targetPath, ...args) { + const stats = originalLstatSync.call(fs, targetPath, ...args); + if (targetPath !== settingsPath) return stats; + const mismatchedDev = typeof stats.dev === 'bigint' ? stats.dev + 1n : stats.dev + 1; + return deriveStats(stats, { dev: mismatchedDev }); + }; + + assert.throws( + () => updateSettingsAtomic( + settingsPath, + settings => ({ settings: { ...settings, managed: true } }) + ), + error => error.code === 'ECC_SETTINGS_CHANGED' + ); + assert.strictEqual(fs.readFileSync(settingsPath, 'utf8'), initial); + assert.ok(!fs.existsSync(`${settingsPath}.ecc.lock`)); + } finally { + fs.lstatSync = originalLstatSync; + Object.defineProperty(process, 'platform', { + value: originalPlatform, + configurable: true, + }); + fs.rmSync(tempDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('settings snapshots request BigInt stats and reject inodes that collide as Numbers', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-bigint-identity-')); + const settingsPath = path.join(tempDir, 'settings.json'); + const originalOpenSync = fs.openSync; + const originalFstatSync = fs.fstatSync; + const originalLstatSync = fs.lstatSync; + let settingsDescriptor; + let sawBigIntFstat = false; + let sawBigIntLstat = false; + const descriptorIno = 9007199254740992n; + const pathIno = 9007199254740993n; + try { + fs.writeFileSync(settingsPath, '{"theme":"initial"}\n'); + fs.openSync = function(targetPath, ...args) { + const descriptor = originalOpenSync.call(fs, targetPath, ...args); + if (targetPath === settingsPath) settingsDescriptor = descriptor; + return descriptor; + }; + fs.fstatSync = function(descriptor, options) { + const stats = originalFstatSync.call(fs, descriptor, options); + if (descriptor !== settingsDescriptor) return stats; + sawBigIntFstat = options && options.bigint === true; + return deriveStats(stats, { + ino: typeof stats.ino === 'bigint' ? descriptorIno : Number(descriptorIno), + }); + }; + fs.lstatSync = function(targetPath, options) { + const stats = originalLstatSync.call(fs, targetPath, options); + if (targetPath !== settingsPath) return stats; + sawBigIntLstat = options && options.bigint === true; + return deriveStats(stats, { + ino: typeof stats.ino === 'bigint' ? pathIno : Number(pathIno), + }); + }; + + assert.throws( + () => updateSettingsAtomic( + settingsPath, + settings => ({ settings: { ...settings, managed: true } }) + ), + error => error.code === 'ECC_SETTINGS_CHANGED' + ); + assert.strictEqual(sawBigIntFstat, true); + assert.strictEqual(sawBigIntLstat, true); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(settingsPath, 'utf8')), { + theme: 'initial', + }); + } finally { + fs.openSync = originalOpenSync; + fs.fstatSync = originalFstatSync; + fs.lstatSync = originalLstatSync; + fs.rmSync(tempDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + if (test('atomic settings updates retry after a concurrent change and preserve secure mode', () => { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-atomic-')); const settingsPath = path.join(tempDir, 'settings.json'); @@ -429,6 +603,77 @@ function runTests() { } })) passed++; else failed++; + if (test('settings lock release preserves a lock with an unequal nonzero Windows device id', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-release-dev-')); + const settingsPath = path.join(tempDir, 'settings.json'); + const lockPath = `${settingsPath}.ecc.lock`; + const originalLstatSync = fs.lstatSync; + const originalPlatform = process.platform; + let lockContents; + try { + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + fs.lstatSync = function(targetPath, ...args) { + const stats = originalLstatSync.call(fs, targetPath, ...args); + if (!String(targetPath).includes('.ecc.lock.release-')) return stats; + const mismatchedDev = typeof stats.dev === 'bigint' ? stats.dev + 1n : stats.dev + 1; + return deriveStats(stats, { dev: mismatchedDev }); + }; + + assert.throws( + () => runWithSettingsLock(settingsPath, () => { + lockContents = fs.readFileSync(lockPath, 'utf8'); + }), + /Refusing to release a changed Claude settings lock/ + ); + assert.strictEqual(fs.readFileSync(lockPath, 'utf8'), lockContents); + } finally { + fs.lstatSync = originalLstatSync; + Object.defineProperty(process, 'platform', { + value: originalPlatform, + configurable: true, + }); + fs.rmSync(tempDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('stale lock recovery preserves a lock with an unequal nonzero Windows device id', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-stale-dev-')); + const settingsPath = path.join(tempDir, 'settings.json'); + const lockPath = `${settingsPath}.ecc.lock`; + const originalLstatSync = fs.lstatSync; + const originalPlatform = process.platform; + const lockContents = 'foreign stale lock\n'; + try { + fs.writeFileSync(lockPath, lockContents, { mode: 0o600 }); + const stale = new Date(Date.now() - (10 * 60 * 1000)); + fs.utimesSync(lockPath, stale, stale); + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + fs.lstatSync = function(targetPath, ...args) { + const stats = originalLstatSync.call(fs, targetPath, ...args); + if (!stats || !String(targetPath).includes('.ecc.lock.stale-')) return stats; + const mismatchedDev = typeof stats.dev === 'bigint' ? stats.dev + 1n : stats.dev + 1; + return deriveStats(stats, { dev: mismatchedDev }); + }; + + assert.throws( + () => updateSettingsAtomic( + settingsPath, + settings => ({ settings: { ...settings, recovered: true } }) + ), + /Another ECC process is updating Claude settings/ + ); + assert.strictEqual(fs.readFileSync(lockPath, 'utf8'), lockContents); + assert.ok(!fs.existsSync(`${lockPath}.recover`)); + } finally { + fs.lstatSync = originalLstatSync; + Object.defineProperty(process, 'platform', { + value: originalPlatform, + configurable: true, + }); + fs.rmSync(tempDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + if (test('atomic settings updates refuse a symlinked destination', () => { if (process.platform === 'win32') { console.log(' (file symlink support is environment-dependent on Windows; skipping)'); diff --git a/tests/scripts/memory-mcp.test.js b/tests/scripts/memory-mcp.test.js index 9adf37bf5..5698f93e1 100644 --- a/tests/scripts/memory-mcp.test.js +++ b/tests/scripts/memory-mcp.test.js @@ -260,6 +260,7 @@ async function withClient(fn, options = {}) { { name, arguments: toolArguments } ), callToolRaw: params => request('tools/call', params), + ping: params => request('ping', params), }; phase = 'callback'; await Promise.race([Promise.resolve().then(() => fn(client, fixture)), transportFailure]); @@ -393,6 +394,25 @@ async function main() { }); }); + await test('accepts the reserved _meta param on ping and rejects malformed values (#2810)', async () => { + await withClient(async client => { + assert.deepStrictEqual(await client.ping({ _meta: { progressToken: 'progress-1' } }), {}); + assert.deepStrictEqual(await client.ping(), {}); + assert.deepStrictEqual(await client.ping({}), {}); + + for (const badMeta of [null, ['not', 'an', 'object'], 'string', 42, true]) { + await assert.rejects( + client.ping({ _meta: badMeta }), + /-32602/, + `expected ping _meta=${JSON.stringify(badMeta)} to be rejected` + ); + } + + await assert.rejects(client.ping({ unexpected: true }), /-32602/); + await assert.rejects(client.ping({ _meta: {}, unexpected: true }), /-32602/); + }); + }); + await test('accepts the reserved _meta param on tools/call and rejects malformed values', async () => { await withClient(async client => { // A valid `_meta` object (e.g. progressToken) must not block the tool call.