-
-
Notifications
You must be signed in to change notification settings - Fork 2
Description
Description
The file storage implementation in src/storage/file.ts
has a TODO comment at line 32 about implementing atomic file writes. Currently, the file is written directly which could lead to corruption if the process crashes or multiple processes access the file simultaneously.
What needs to be done
Implement atomic file writing using the temp file + rename pattern:
- Write data to a temporary file in the same directory
- Ensure the temp file is fully written and synced to disk
- Atomically rename the temp file to the target filename
- Handle cleanup of temp files in case of errors
Why this matters
Token storage is critical for OAuth security. File corruption could lead to:
- Loss of refresh tokens (requiring users to re-authenticate)
- Partial writes creating invalid JSON
- Race conditions in multi-process scenarios
- Security vulnerabilities if tokens are partially written
Implementation considerations
-
Platform differences: Atomic rename behavior varies between Windows and Unix systems. Should we use a library like
write-file-atomic
or implement platform-specific logic? -
Performance impact: Atomic writes are slower. Should this be configurable for users who don't need atomic guarantees?
-
Alternative approach: Consider using a lightweight embedded database (SQLite) or lock files instead of JSON files for better concurrency handling
-
Backup strategy: Should we keep a backup of the previous file in case the new write fails validation?
-
File permissions: Ensure temp files have the same permissions as the target file (important for security)
Code reference
Current implementation at src/storage/file.ts:30-34
:
async function writeStore(data: Record<string, Tokens>) {
await ensureDir();
// TODO: Atomic write via temp file + rename
await fs.writeFile(file, JSON.stringify(data, null, 2), "utf-8");
}
Suggested implementation approach
async function writeStore(data: Record<string, Tokens>) {
await ensureDir();
const tempFile = `${file}.tmp.${process.pid}.${Date.now()}`;
try {
// Write to temp file
await fs.writeFile(tempFile, JSON.stringify(data, null, 2), "utf-8");
// Ensure data is flushed to disk (platform-specific)
const fd = await fs.open(tempFile, 'r');
await fd.sync();
await fd.close();
// Atomic rename
await fs.rename(tempFile, file);
} catch (error) {
// Clean up temp file on error
try {
await fs.unlink(tempFile);
} catch {
// Ignore cleanup errors
}
throw error;
}
}
Testing requirements
- Test concurrent writes don't corrupt the file
- Test process interruption scenarios
- Test disk full conditions
- Verify temp files are cleaned up
Skills required
- TypeScript
- Node.js fs module
- File system operations
- Understanding of atomic operations
Difficulty
Medium - Requires understanding of file system atomicity and error handling