fix(accounts): remove a rejected GLM key when restore fails

A failed plaintext replacement can publish the new key before file
restriction is confirmed. If restoring the previous key fails before it
replaces that file, delete only the rejected envelope so a completed
restore is left intact.
This commit is contained in:
Luchong
2026-10-10 16:24:14 -07:00
committed by Neil
parent 9029e84883
commit c33fa877d4
2 changed files with 54 additions and 7 deletions
@@ -170,6 +170,43 @@ describe('zcode-plan-api-key-store', () => {
warn.mockRestore()
})
it('removes a rejected plaintext replacement when restoring the previous key fails', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined)
safeStorageMock.isEncryptionAvailable.mockReturnValue(false)
const previous = Buffer.from(envelope('encrypted', 'old-key'))
const attempted = envelope('plaintext', 'new-key')
existsSyncMock.mockReturnValue(true)
readFileSyncMock.mockReturnValueOnce(previous).mockReturnValueOnce(Buffer.from(attempted))
writeSecureFileMock.mockReturnValueOnce(false).mockImplementationOnce(() => {
throw new Error('restore failed before publish')
})
const store = await loadStore()
expect(() => store.saveZcodePlanApiKey('new-key')).toThrow(
'could not be stored securely on this device'
)
expect(rmSyncMock).toHaveBeenCalledWith(storePath, { force: true })
warn.mockRestore()
})
it('keeps a restored key when rollback fails after the previous envelope is published', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined)
safeStorageMock.isEncryptionAvailable.mockReturnValue(false)
const previous = Buffer.from(envelope('encrypted', 'old-key'))
existsSyncMock.mockReturnValue(true)
readFileSyncMock.mockReturnValue(previous)
writeSecureFileMock.mockReturnValueOnce(false).mockImplementationOnce(() => {
throw new Error('restore failed after publish')
})
const store = await loadStore()
expect(() => store.saveZcodePlanApiKey('new-key')).toThrow(
'could not be stored securely on this device'
)
expect(rmSyncMock).not.toHaveBeenCalledWith(storePath, expect.anything())
warn.mockRestore()
})
it('refuses to decrypt an encrypted envelope once safeStorage becomes unavailable', async () => {
safeStorageMock.isEncryptionAvailable.mockReturnValue(false)
existsSyncMock.mockReturnValue(true)
+17 -7
View File
@@ -84,6 +84,19 @@ export function getZcodePlanApiKeyProtection(): SecretAtRestProtection | null {
}
}
function removeRejectedPlaintextEnvelope(keyPath: string, attemptedEnvelope: string): void {
let published: string
try {
published = readFileSync(keyPath).toString('utf8')
} catch {
return
}
// Why: only the rejected replacement is removed; a restore that already landed must stay.
if (published === attemptedEnvelope) {
rmSync(keyPath, { force: true })
}
}
export function saveZcodePlanApiKey(key: string): void {
const trimmed = key.trim()
if (!trimmed) {
@@ -115,10 +128,8 @@ export function saveZcodePlanApiKey(key: string): void {
previousEnvelope = null
}
}
const wroteRestricted = writeSecureFile(
keyPath,
encodeApiKeyEnvelope('plaintext', Buffer.from(trimmed, 'utf8'))
)
const attemptedEnvelope = encodeApiKeyEnvelope('plaintext', Buffer.from(trimmed, 'utf8'))
const wroteRestricted = writeSecureFile(keyPath, attemptedEnvelope)
// Why: an unrestricted plaintext credential must never be reported as saved.
if (!wroteRestricted) {
if (!previousEnvelope) {
@@ -127,9 +138,8 @@ export function saveZcodePlanApiKey(key: string): void {
try {
writeSecureFile(keyPath, previousEnvelope.toString('utf8'))
} catch {
// Why: restriction is failing device-wide; the restored bytes keep the
// previous credential available instead of deleting it, and the thrown
// save error still tells the user the store is not secure.
// Why: a restore that fails before replacing the file leaves the rejected key published.
removeRejectedPlaintextEnvelope(keyPath, attemptedEnvelope)
}
}
throw new Error('GLM Coding Plan API key could not be stored securely on this device')