Skip to content

Commit c3659cf

Browse files
authored
feat(config): use EditableJson for non-destructive config saving (#997)
* feat(config): use EditableJson for non-destructive config saving Use EditableJson for preserving existing properties and key order when updating config values. This prevents overwriting unrelated config properties during partial updates. - Add standalone EditableJson implementation in src/utils/editable-json.mts - Update config.mts to use EditableJson for config file writes - Fix socketAppDataPath usage to include config.json filename - Add resetConfigForTesting() helper for test isolation - Update tests to use Node.js built-in fs functions * fix: address PR review feedback - Preserve JSON formatting by using editor's indent/newline symbols - Handle deleted config keys explicitly (editor.update only merges) - Capture config snapshot at write time, not schedule time - Fix load method create parameter to actually create empty state * fix: TypeScript compilation errors - Fix symbol index type errors with type assertions - Remove unused getDefaultFormatting function - Add null check for match[1] in detectIndent - Remove unused contentWithoutImport variable * fix: ESLint errors in config and editable-json - Fix import sort order in config.test.mts (promises as fs sorted by alias name) - Add no-await-in-loop eslint-disable comments for retry loops in editable-json.mts - Remove unused eslint-disable directive * chore: remove unused imports from config.test.mts
1 parent 1de47a9 commit c3659cf

File tree

3 files changed

+463
-10
lines changed

3 files changed

+463
-10
lines changed

src/utils/config.mts

Lines changed: 80 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import { logger } from '@socketsecurity/registry/lib/logger'
3131
import { naturalCompare } from '@socketsecurity/registry/lib/sorts'
3232

3333
import { debugConfig } from './debug.mts'
34+
import { getEditableJsonClass } from './editable-json.mts'
3435
import constants, {
3536
CONFIG_KEY_API_BASE_URL,
3637
CONFIG_KEY_API_PROXY,
@@ -98,17 +99,18 @@ function getConfigValues(): LocalConfig {
9899
_cachedConfig = {} as LocalConfig
99100
const { socketAppDataPath } = constants
100101
if (socketAppDataPath) {
101-
const raw = safeReadFileSync(socketAppDataPath)
102+
const configFilePath = path.join(socketAppDataPath, 'config.json')
103+
const raw = safeReadFileSync(configFilePath)
102104
if (raw) {
103105
try {
104106
Object.assign(
105107
_cachedConfig,
106108
JSON.parse(Buffer.from(raw, 'base64').toString()),
107109
)
108-
debugConfig(socketAppDataPath, true)
110+
debugConfig(configFilePath, true)
109111
} catch (e) {
110-
logger.warn(`Failed to parse config at ${socketAppDataPath}`)
111-
debugConfig(socketAppDataPath, false, e)
112+
logger.warn(`Failed to parse config at ${configFilePath}`)
113+
debugConfig(configFilePath, false, e)
112114
}
113115
// Normalize apiKey to apiToken and persist it.
114116
// This is a one time migration per user.
@@ -118,7 +120,7 @@ function getConfigValues(): LocalConfig {
118120
updateConfigValue(CONFIG_KEY_API_TOKEN, token)
119121
}
120122
} else {
121-
mkdirSync(path.dirname(socketAppDataPath), { recursive: true })
123+
mkdirSync(socketAppDataPath, { recursive: true })
122124
}
123125
}
124126
}
@@ -243,6 +245,16 @@ let _cachedConfig: LocalConfig | undefined
243245
// When using --config or SOCKET_CLI_CONFIG, do not persist the config.
244246
let _configFromFlag = false
245247

248+
/**
249+
* Reset config cache for testing purposes.
250+
* This allows tests to start with a fresh config state.
251+
* @internal
252+
*/
253+
export function resetConfigForTesting(): void {
254+
_cachedConfig = undefined
255+
_configFromFlag = false
256+
}
257+
246258
export function overrideCachedConfig(jsonConfig: unknown): CResult<undefined> {
247259
debugFn('notice', 'override: full config (not stored)')
248260

@@ -340,11 +352,72 @@ export function updateConfigValue<Key extends keyof LocalConfig>(
340352
_pendingSave = true
341353
process.nextTick(() => {
342354
_pendingSave = false
355+
// Capture the config state at write time, not at schedule time.
356+
// This ensures all updates in the same tick are included.
357+
const configToSave = { ...localConfig }
343358
const { socketAppDataPath } = constants
344359
if (socketAppDataPath) {
360+
mkdirSync(socketAppDataPath, { recursive: true })
361+
const configFilePath = path.join(socketAppDataPath, 'config.json')
362+
// Read existing file to preserve formatting, then update with new values.
363+
const existingRaw = safeReadFileSync(configFilePath)
364+
const EditableJson = getEditableJsonClass<LocalConfig>()
365+
const editor = new EditableJson()
366+
if (existingRaw !== undefined) {
367+
const rawString = Buffer.isBuffer(existingRaw)
368+
? existingRaw.toString('utf8')
369+
: existingRaw
370+
try {
371+
const decoded = Buffer.from(rawString, 'base64').toString('utf8')
372+
editor.fromJSON(decoded)
373+
} catch {
374+
// If decoding fails, start fresh.
375+
}
376+
} else {
377+
// Initialize empty editor for new file.
378+
editor.create(configFilePath)
379+
}
380+
// Update with the captured config state.
381+
// Note: We need to handle deletions explicitly since editor.update() only merges.
382+
// First, get all keys from the existing content.
383+
const existingKeys = new Set(
384+
Object.keys(editor.content).filter(k => typeof k === 'string'),
385+
)
386+
const newKeys = new Set(Object.keys(configToSave))
387+
388+
// Delete keys that are in existing but not in new config.
389+
for (const key of existingKeys) {
390+
if (!newKeys.has(key)) {
391+
delete (editor.content as any)[key]
392+
}
393+
}
394+
395+
// Now update with new values.
396+
editor.update(configToSave)
397+
// Use the editor's internal stringify which preserves formatting.
398+
// Extract the formatting symbols from the content.
399+
const INDENT_SYMBOL = Symbol.for('indent')
400+
const NEWLINE_SYMBOL = Symbol.for('newline')
401+
const indent = (editor.content as any)[INDENT_SYMBOL] ?? 2
402+
const newline = (editor.content as any)[NEWLINE_SYMBOL] ?? '\n'
403+
404+
// Strip formatting symbols from content.
405+
const contentToSave: Record<string, unknown> = {}
406+
for (const [key, val] of Object.entries(editor.content)) {
407+
if (typeof key === 'string') {
408+
contentToSave[key] = val
409+
}
410+
}
411+
412+
// Stringify with formatting preserved.
413+
const jsonContent = JSON.stringify(
414+
contentToSave,
415+
undefined,
416+
indent,
417+
).replace(/\n/g, newline)
345418
writeFileSync(
346-
socketAppDataPath,
347-
Buffer.from(JSON.stringify(localConfig)).toString('base64'),
419+
configFilePath,
420+
Buffer.from(jsonContent + newline).toString('base64'),
348421
)
349422
}
350423
})

src/utils/config.test.mts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
1-
import { promises as fs, mkdtempSync } from 'node:fs'
1+
import {
2+
promises as fs,
3+
mkdtempSync,
4+
readFileSync,
5+
rmSync,
6+
writeFileSync,
7+
} from 'node:fs'
28
import os from 'node:os'
39
import path from 'node:path'
410

5-
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
11+
import { beforeEach, describe, expect, it } from 'vitest'
612

713
import {
814
findSocketYmlSync,
@@ -80,7 +86,7 @@ describe('utils/config', () => {
8086
expect(result.data).toBe(undefined)
8187
} finally {
8288
// Clean up the temporary directory.
83-
await fs.rm(tmpDir, { force: true, recursive: true })
89+
rmSync(tmpDir, { force: true, recursive: true })
8490
}
8591
})
8692
})

0 commit comments

Comments
 (0)