Skip to content

Commit e3c948b

Browse files
fix(auth): add automatic key reload for state directory recovery (#3374)
Closes #3367. Adds reload() with mtime-based detection, auto-reload on validation failure, and isHealthy() for degraded state detection.
1 parent dee2da9 commit e3c948b

3 files changed

Lines changed: 162 additions & 2 deletions

File tree

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/**
2+
* Issue #3367: Server can enter unrecoverable auth state where all tokens are invalid.
3+
*
4+
* After state directory churn (delete + ag init --force), the server should
5+
* automatically recover by re-reading keys.json from disk on the next
6+
* validation failure.
7+
*/
8+
9+
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
10+
import { mkdir, writeFile, rm } from 'node:fs/promises';
11+
import { statSync } from 'node:fs';
12+
import { join } from 'node:path';
13+
import { AuthManager } from '../services/auth/AuthManager.js';
14+
15+
describe('Issue #3367: Auth state recovery after state directory loss', () => {
16+
let tmpDir: string;
17+
let keysFile: string;
18+
let auth: AuthManager;
19+
20+
beforeEach(async () => {
21+
tmpDir = join('/tmp', `test-3367-${Date.now()}-${Math.random().toString(36).slice(2)}`);
22+
await mkdir(tmpDir, { recursive: true });
23+
keysFile = join(tmpDir, 'keys.json');
24+
auth = new AuthManager(keysFile, 'master-secret');
25+
});
26+
27+
afterEach(async () => {
28+
await rm(tmpDir, { recursive: true, force: true });
29+
});
30+
31+
it('reload() picks up new keys after state dir wipe and recreate', async () => {
32+
// Step 1: Create initial key
33+
const { key: oldKey } = await auth.createKey('old-key', 100, undefined, 'admin');
34+
expect(auth.validate(oldKey).valid).toBe(true);
35+
36+
// Step 2: Delete keys.json (simulate state dir wipe)
37+
await rm(keysFile);
38+
39+
// Step 3: Old key should still work (in-memory)
40+
expect(auth.validate(oldKey).valid).toBe(true);
41+
42+
// Step 4: Write new keys.json (simulate ag init --force creating new token)
43+
const newAuth = new AuthManager(keysFile, 'master-secret');
44+
const { key: newKey } = await newAuth.createKey('new-key', 100, undefined, 'admin');
45+
await newAuth.save();
46+
47+
// Step 5: Old key still works (in-memory), new key doesn't
48+
expect(auth.validate(oldKey).valid).toBe(true);
49+
expect(auth.validate(newKey).valid).toBe(false);
50+
51+
// Step 6: Reload should pick up new keys
52+
const reloaded = await auth.reload();
53+
expect(reloaded).toBe(true);
54+
55+
// Step 7: New key should now work, old key should not
56+
expect(auth.validate(newKey).valid).toBe(true);
57+
expect(auth.validate(oldKey).valid).toBe(false);
58+
});
59+
60+
it('reload() does nothing if keys.json has not changed', async () => {
61+
await auth.createKey('test-key', 100, undefined, 'admin');
62+
await auth.load(); // ensure mtime is tracked
63+
const reloaded = await auth.reload();
64+
expect(reloaded).toBe(false);
65+
});
66+
67+
it('reload() handles missing keys.json gracefully', async () => {
68+
const { key: existingKey } = await auth.createKey('graceful-key', 100, undefined, 'admin');
69+
expect(auth.validate(existingKey).valid).toBe(true);
70+
await rm(keysFile);
71+
const reloaded = await auth.reload();
72+
expect(reloaded).toBe(false);
73+
expect(auth.validate(existingKey).valid).toBe(true);
74+
expect(auth.isHealthy()).toBe(false);
75+
});
76+
77+
it('reload() is safe to call concurrently', async () => {
78+
await auth.createKey('test-key', 100, undefined, 'admin');
79+
const results = await Promise.all([auth.reload(), auth.reload(), auth.reload()]);
80+
expect(results.every(r => typeof r === 'boolean')).toBe(true);
81+
});
82+
83+
it('isHealthy() returns false when keys.json missing but keys exist in memory', async () => {
84+
await auth.createKey('test-key', 100, undefined, 'admin');
85+
expect(auth.isHealthy()).toBe(true);
86+
await rm(keysFile);
87+
expect(auth.isHealthy()).toBe(false);
88+
});
89+
90+
it('isHealthy() returns true when keys.json exists', async () => {
91+
await auth.createKey('test-key', 100, undefined, 'admin');
92+
expect(auth.isHealthy()).toBe(true);
93+
});
94+
95+
it('isHealthy() returns true when no keys and no file (fresh start)', () => {
96+
expect(auth.isHealthy()).toBe(true);
97+
});
98+
99+
it('save() updates mtime so subsequent reload() is a no-op', async () => {
100+
await auth.createKey('key1', 100, undefined, 'admin');
101+
const mtimeAfterSave = statSync(keysFile).mtimeMs;
102+
const reloaded = await auth.reload();
103+
expect(reloaded).toBe(false);
104+
});
105+
});

src/server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -902,7 +902,7 @@ async function main(): Promise<void> {
902902
await auth.load();
903903
},
904904
stop: async () => {},
905-
health: async () => ({ healthy: true }),
905+
health: async () => ({ healthy: auth.isHealthy(), details: auth.isHealthy() ? undefined : "keys.json missing — state dir may have been wiped" }),
906906
});
907907
container.register('channelManager', channels, {
908908
start: async () => {

src/services/auth/AuthManager.ts

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { createHash, randomBytes } from 'node:crypto';
1010
import { timingSafeStringEqual } from '../../crypto-utils.js';
1111
import { readFile, writeFile, mkdir, rename } from 'node:fs/promises';
1212
import { authStoreSchema } from '../../validation.js';
13-
import { existsSync } from 'node:fs';
13+
import { existsSync, statSync } from 'node:fs';
1414
import { dirname } from 'node:path';
1515
import { secureFilePermissions } from '../../file-utils.js';
1616
import { SYSTEM_TENANT } from '../../config.js';
@@ -86,6 +86,10 @@ export class AuthManager {
8686
private audit: AuditLogger | null = null;
8787
/** #2534: Dirty flag — set when lastUsedAt is updated, cleared when persisted to disk. */
8888
private lastUsedAtDirty = false;
89+
/** #3367: Track mtime of keys.json to detect external changes. */
90+
private lastKeysMtime: number | null = null;
91+
/** #3367: Guard against concurrent reloads. */
92+
private reloading = false;
8993

9094

9195
constructor(
@@ -136,6 +140,9 @@ export class AuthManager {
136140
await this.save();
137141
}
138142
}
143+
// #3367: Track mtime after load
144+
try { this.lastKeysMtime = statSync(this.keysFile).mtimeMs; } catch { /* ignore */ }
145+
139146
// Issue #2097: Load grace keys from persisted store
140147
if (Array.isArray(parsed.graceKeys)) {
141148
const now = Date.now();
@@ -227,6 +234,8 @@ export class AuthManager {
227234
await writeFile(tmpFile, JSON.stringify(data, null, 2), { mode: 0o600 });
228235
await rename(tmpFile, this.keysFile);
229236
await secureFilePermissions(this.keysFile);
237+
// #3367: Track mtime after save
238+
try { this.lastKeysMtime = statSync(this.keysFile).mtimeMs; } catch { /* ignore */ }
230239
}
231240

232241
/** Create a new API key. Returns the plaintext key (only shown once). */
@@ -535,6 +544,11 @@ export class AuthManager {
535544
return { valid: true, keyId: rotatedKey.id, rateLimited: false, tenantId: rotatedKey.tenantId };
536545
}
537546
}
547+
// #3367: Trigger async reload on invalid — recovers from state dir wipe
548+
setImmediate(async () => {
549+
const reloaded = await this.reload();
550+
if (reloaded) console.warn('[AuthManager] Keys reloaded after failed validation');
551+
});
538552
return { valid: false, keyId: null, rateLimited: false, reason: 'invalid' };
539553
}
540554

@@ -613,6 +627,47 @@ export class AuthManager {
613627
return key?.tenantId;
614628
}
615629

630+
/** #3367: Reload keys from disk if the file has changed since last load/save. */
631+
async reload(): Promise<boolean> {
632+
if (this.reloading) return false;
633+
this.reloading = true;
634+
try {
635+
if (!existsSync(this.keysFile)) {
636+
console.warn('[AuthManager] keys.json disappeared — keeping in-memory keys');
637+
this.lastKeysMtime = null;
638+
return false;
639+
}
640+
if (!this._keysFileChanged()) return false;
641+
console.warn('[AuthManager] keys.json changed on disk — reloading');
642+
await this.load();
643+
return true;
644+
} finally {
645+
this.reloading = false;
646+
}
647+
}
648+
649+
/** #3367: Check if keys.json mtime has changed. */
650+
private _keysFileChanged(): boolean {
651+
try {
652+
const currentMtime = statSync(this.keysFile).mtimeMs;
653+
if (this.lastKeysMtime === null || currentMtime > this.lastKeysMtime) {
654+
this.lastKeysMtime = currentMtime;
655+
return true;
656+
}
657+
return false;
658+
} catch {
659+
return false;
660+
}
661+
}
662+
663+
/** #3367: Health check — false if keys.json missing but in-memory keys exist. */
664+
isHealthy(): boolean {
665+
if (this.store.keys.length > 0 && !existsSync(this.keysFile)) {
666+
return false;
667+
}
668+
return true;
669+
}
670+
616671
/** Hash a key with SHA-256. */
617672
static hashKey(key: string): string {
618673
return createHash('sha256').update(key).digest('hex');

0 commit comments

Comments
 (0)