diff --git a/docs/reports/hardening-inventory.json b/docs/reports/hardening-inventory.json index 1f1a0273..baf2e2d6 100644 --- a/docs/reports/hardening-inventory.json +++ b/docs/reports/hardening-inventory.json @@ -1,31 +1,11 @@ { "scope": "src/**/*.{ts,tsx,js,jsx,mjs,cjs}", "syncFs": { - "totalOccurrences": 2516, + "totalOccurrences": 2509, "filesAffected": 263, - "hotpathOccurrences": 1190, + "hotpathOccurrences": 1183, "hotpathFilesAffected": 156, "topHotpathFiles": [ - { - "file": "src/management/shared-manager/diverged-file-adopter.ts", - "count": 36, - "calls": [ - "chmodSync", - "closeSync", - "fsyncSync", - "linkSync", - "lstatSync", - "openSync", - "readdirSync", - "readFileSync", - "readlinkSync", - "renameSync", - "statSync", - "unlinkSync", - "writeFileSync" - ], - "markers": [] - }, { "file": "src/utils/browser/mcp-installer.ts", "count": 32, @@ -58,6 +38,26 @@ ], "markers": [] }, + { + "file": "src/management/shared-manager/diverged-file-adopter.ts", + "count": 29, + "calls": [ + "chmodSync", + "closeSync", + "fsyncSync", + "linkSync", + "lstatSync", + "openSync", + "readdirSync", + "readFileSync", + "readlinkSync", + "renameSync", + "statSync", + "unlinkSync", + "writeFileSync" + ], + "markers": [] + }, { "file": "src/utils/claude-symlink-manager.ts", "count": 27, @@ -259,26 +259,6 @@ ], "markers": [] }, - { - "file": "src/management/shared-manager/diverged-file-adopter.ts", - "count": 36, - "calls": [ - "chmodSync", - "closeSync", - "fsyncSync", - "linkSync", - "lstatSync", - "openSync", - "readdirSync", - "readFileSync", - "readlinkSync", - "renameSync", - "statSync", - "unlinkSync", - "writeFileSync" - ], - "markers": [] - }, { "file": "src/cliproxy/executor/__tests__/composite-variant-service.test.ts", "count": 33, @@ -304,6 +284,22 @@ "writeFileSync" ], "markers": [] + }, + { + "file": "src/utils/browser/mcp-installer.ts", + "count": 32, + "calls": [ + "chmodSync", + "copyFileSync", + "existsSync", + "mkdirSync", + "readFileSync", + "renameSync", + "statSync", + "unlinkSync", + "writeFileSync" + ], + "markers": [] } ] }, diff --git a/docs/reports/hardening-inventory.md b/docs/reports/hardening-inventory.md index 43e4b23c..0753174f 100644 --- a/docs/reports/hardening-inventory.md +++ b/docs/reports/hardening-inventory.md @@ -6,9 +6,9 @@ Scope: `src/**/*.{ts,tsx,js,jsx,mjs,cjs}` | Metric | Value | |---|---:| -| Sync fs occurrences (all) | 2516 | +| Sync fs occurrences (all) | 2509 | | Sync fs files affected (all) | 263 | -| Sync fs occurrences (runtime hotpaths) | 1190 | +| Sync fs occurrences (runtime hotpaths) | 1183 | | Sync fs files affected (runtime hotpaths) | 156 | | Legacy shim markers | 465 | | Legacy shim files affected | 176 | @@ -17,9 +17,9 @@ Scope: `src/**/*.{ts,tsx,js,jsx,mjs,cjs}` | File | Sync Calls | API Names | |---|---:|---| -| `src/management/shared-manager/diverged-file-adopter.ts` | 36 | chmodSync, closeSync, fsyncSync, linkSync, lstatSync, openSync, readdirSync, readFileSync, readlinkSync, renameSync, statSync, unlinkSync, writeFileSync | | `src/utils/browser/mcp-installer.ts` | 32 | chmodSync, copyFileSync, existsSync, mkdirSync, readFileSync, renameSync, statSync, unlinkSync, writeFileSync | | `src/utils/image-analysis/mcp-installer.ts` | 30 | chmodSync, copyFileSync, existsSync, mkdirSync, readFileSync, renameSync, statSync, unlinkSync, writeFileSync | +| `src/management/shared-manager/diverged-file-adopter.ts` | 29 | chmodSync, closeSync, fsyncSync, linkSync, lstatSync, openSync, readdirSync, readFileSync, readlinkSync, renameSync, statSync, unlinkSync, writeFileSync | | `src/utils/claude-symlink-manager.ts` | 27 | copyFileSync, existsSync, lstatSync, mkdirSync, readdirSync, readlinkSync, renameSync, rmSync, statSync, symlinkSync, unlinkSync | | `src/cliproxy/config/env-builder.ts` | 25 | existsSync, mkdirSync, readFileSync, writeFileSync | | `src/management/shared-manager/migrations.ts` | 25 | copyFileSync, cpSync, existsSync, lstatSync, mkdirSync, readdirSync, symlinkSync, unlinkSync, writeFileSync | diff --git a/src/management/shared-manager/diverged-file-adopter.ts b/src/management/shared-manager/diverged-file-adopter.ts index 41dbde84..2e42f7e2 100644 --- a/src/management/shared-manager/diverged-file-adopter.ts +++ b/src/management/shared-manager/diverged-file-adopter.ts @@ -164,6 +164,36 @@ function canonicalIdentityMatches( ); } +function tempWritePath(targetPath: string): string { + return `${targetPath}.ccs-write-${process.pid}-${Date.now()}-${adoptionClaimSequence++}`; +} + +/** + * Write content into a fresh temp file and make it durable, so whatever + * publishes it under its final name publishes complete bytes. + */ +function writeDurableTempFile(tempPath: string, content: Buffer, mode: number): void { + let descriptor: number | null = null; + try { + descriptor = fs.openSync(tempPath, 'wx', mode); + fs.fchmodSync(descriptor, mode); + fs.writeFileSync(descriptor, content); + fs.fsyncSync(descriptor); + } finally { + if (descriptor !== null) { + fs.closeSync(descriptor); + } + } +} + +function discardTempFile(tempPath: string): void { + try { + fs.unlinkSync(tempPath); + } catch { + // The temp file may not have been created or may already have been renamed. + } +} + /** * Create a file only if the path is free, writing the content atomically. * @@ -171,26 +201,13 @@ function canonicalIdentityMatches( * cannot overwrite each other's sidecar artifacts. */ function createFileNoReplace(targetPath: string, content: Buffer, mode: number): void { - const tempPath = `${targetPath}.ccs-write-${process.pid}-${Date.now()}-${adoptionClaimSequence++}`; - let descriptor: number | null = null; + const tempPath = tempWritePath(targetPath); try { - descriptor = fs.openSync(tempPath, 'wx', mode); - fs.fchmodSync(descriptor, mode); - fs.writeFileSync(descriptor, content); - fs.fsyncSync(descriptor); - fs.closeSync(descriptor); - descriptor = null; + writeDurableTempFile(tempPath, content, mode); fs.linkSync(tempPath, targetPath); fs.unlinkSync(tempPath); } catch (err) { - if (descriptor !== null) { - fs.closeSync(descriptor); - } - try { - fs.unlinkSync(tempPath); - } catch { - // The temp file may not have been created or may already have been renamed. - } + discardTempFile(tempPath); throw err; } } @@ -226,6 +243,13 @@ function publishSidecarNoReplace(basePath: string, content: Buffer, mode: number * that got there first; the adopted bytes stay in the sidecars the caller * published. A pure chmod is not a content change, so the mode the inode * carries at publication time wins. + * + * The guard is read-then-act, not atomic: POSIX offers no compare-and-swap + * rename, so a writer landing between the check and the rename is still + * overwritten. That is a narrowing, not a guarantee - the window shrinks from + * the ~100 ms the old claim-and-republish path left open to two adjacent + * syscalls, and the pre-image sidecar the caller published keeps even that + * outcome recoverable. Do not build stricter guarantees on top of it. */ function publishCanonicalContent( writePath: string, @@ -233,15 +257,9 @@ function publishCanonicalContent( mode: number, expected: CanonicalIdentity | null ): void { - const tempPath = `${writePath}.ccs-write-${process.pid}-${Date.now()}-${adoptionClaimSequence++}`; - let descriptor: number | null = null; + const tempPath = tempWritePath(writePath); try { - descriptor = fs.openSync(tempPath, 'wx', mode); - fs.fchmodSync(descriptor, mode); - fs.writeFileSync(descriptor, content); - fs.fsyncSync(descriptor); - fs.closeSync(descriptor); - descriptor = null; + writeDurableTempFile(tempPath, content, mode); const currentStats = getLstatSync(writePath); const current = currentStats?.isFile() ? canonicalIdentityOf(currentStats) : null; @@ -257,14 +275,7 @@ function publishCanonicalContent( } fs.renameSync(tempPath, writePath); } catch (err) { - if (descriptor !== null) { - fs.closeSync(descriptor); - } - try { - fs.unlinkSync(tempPath); - } catch { - // The temp file may not have been created or may already have been renamed. - } + discardTempFile(tempPath); throw err; } }